Skip to content

fix: normalize wing in /mine, /backfill-age, watcher auto-mine - #178

Merged
jphein merged 1 commit into
mainfrom
fix/mine-backfill-watcher-wing-normalize
May 29, 2026
Merged

fix: normalize wing in /mine, /backfill-age, watcher auto-mine#178
jphein merged 1 commit into
mainfrom
fix/mine-backfill-watcher-wing-normalize

Conversation

@jphein

@jphein jphein commented May 29, 2026

Copy link
Copy Markdown
Collaborator

Continuation of the wing-canonicalization sweep (#175 added read-side normalization; #177 fixed /silent-save). Three more sites passed wing to subprocess without normalizing:

Site Pre-fix behavior
POST /mine subprocess writes drawers under literal mixed-case wing → unreachable by post-#175 reads
POST /backfill-age uses wing as a filter; mixed-case input never matched the canonical-slug drawers
watcher _internal_mine same shape as /mine; reads wing from PALACE_WATCH_DIRS env (may be mixed-case)

All three now normalize. Together with #175 and #177, this closes the wing-canonicalization sweep: every write surface (/memory, /silent-save, /mine, /backfill-age, watcher) AND every read surface (/search, /search/hybrid, /search/keyword, /search/age-fused, /search/fast, /list) shares the same normalization contract.

Continuation of the wing-canonicalization sweep started in #175 and
#177. Three more sites pass wing to subprocess without normalizing:

1. POST /mine handler — writes drawers via `mempalace mine --wing ...`
   subprocess. Pre-fix: /mine with wing="Palace_Daemon" stored drawers
   under that literal; /search?wing=Palace_Daemon normalizes to
   "palace_daemon" (post-#175) → miss.

2. POST /backfill-age handler — uses wing as a *filter* (restrict
   backfill to one wing). Pre-fix: backfill-age with wing="Palace_Daemon"
   filtered for that literal; drawers stored as "palace_daemon" weren't
   touched.

3. WatcherService._internal_mine — same shape as /mine. Reads wing
   from PALACE_WATCH_DIRS env which may contain mixed-case entries.

All three now route through _normalize_wing_slug (for write paths) or
_rooms.normalize_wing_filter (for the /backfill-age filter path). All
the conceptual variants of "the same wing" now resolve to the same
canonical slug regardless of which endpoint or env var supplied them.

This closes the wing-canonicalization asymmetry sweep: /memory,
/silent-save, /mine, /backfill-age, watcher auto-mine, and the six
read endpoints (/search, /search/hybrid, /search/keyword,
/search/age-fused, /search/fast, /list) all share the same
normalization contract.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 29, 2026 00:49
@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@jphein
jphein merged commit 34cfd15 into main May 29, 2026
@jphein
jphein deleted the fix/mine-backfill-watcher-wing-normalize branch May 29, 2026 00:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

jphein added a commit that referenced this pull request May 29, 2026
…ion (#180)

Implements #179's Option A for the three query-param read endpoints.
After PRs #175/#177/#178 sprinkled normalize/validate calls at 11
sites, this PR moves enforcement up to the request-parse layer for
the simplest subset (query-param endpoints) so future additions get
the contract structurally rather than via per-site convention.

Adds two FastAPI dependency factories in rooms.py:
  - rooms.wing_filter_dep — wraps normalize_wing_filter
  - rooms.room_validator_dep — wraps validate_room_or_raise

Migrates the three query-param read endpoints:
  GET /search       (wing + room)
  GET /list         (wing + room)
  GET /search/fast  (wing only)

Each endpoint declares:
  wing: str | None = Depends(_rooms.wing_filter_dep)
  room: str | None = Depends(_rooms.room_validator_dep)

and the handler body just uses the canonicalized values. The
previous inline canonicalize/validate calls are removed (they would
have been redundant — the dependency runs before the handler).

POST endpoints (/search/hybrid, /search/keyword, /search/age-fused)
stay on inline calls — they parse JSON bodies and would need
pydantic models to use Depends. Filed as a follow-up in #179.

Reordered the `import rooms as _rooms` to top-of-file so the
Depends() references in route signatures resolve at module-load
time (was previously imported in the #101 twelfth-slice block
much later in the file).

Tests: 531 → 534 (3 new dep tests in test_room_validation.py).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jphein added a commit that referenced this pull request May 29, 2026
Updates HANDOFF.md to reflect the full 39-PR session:
- Adds the wing/room canonicalization sweep (#172-#178)
- Adds the #179 architectural follow-up (#180/#181/#182)
- Updates counts: 39 PRs / 534 tests / 30+ deploys
- Notes that #179 is partially complete: 1/5 write surfaces migrated
  to pydantic (BackfillAgeBody via #182); 4 remain with documented
  per-endpoint empty-wing semantics + suggested order

Also adds a section observing the autonomous-loop discovery pattern
that worked this session — "fix the obvious bug, write a curl probe,
sweep for what else has this shape." Documents that the Stop hook's
literal "ongoing imperative" framing is satisfied when marginal value
per cycle drops below context-thrash cost.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jphein added a commit that referenced this pull request May 29, 2026
Third write-surface #179 migration. /mine has the largest body of the
write endpoints (dir, wing, mode, extract, limit) and the most
elaborate validation (mode/extract enum checks, dir filesystem
checks).

Adds MineBody to search_models.py:
  - dir: required string, non-empty (FS checks in handler)
  - wing: empty → "general" then normalized (write-side default,
    matches pre-#178 behavior)
  - mode: enum {convos, projects, session}
  - extract: optional, enum {exchange, general}
  - limit: optional, integer ≥ 1

Filesystem checks on dir (is_absolute, exists, is_dir, no traversal)
stay in the handler — pydantic can't probe the FS at parse time, and
the existing 400 messages name the offending path which is more
useful than a generic pydantic error.

Refactor signature:
  `mine(request, x_api_key)` → `mine(request, body: MineBody, x_api_key)`

The `request` parameter is preserved because `_run_mine_subprocess`
reads `request.app.state.active_mines` for lifespan cleanup
(#138/#139). Pydantic body added alongside.

Test updates: test_mine_backend_aware._request → _request_and_body
returning (Request mock, MineBody). All 6 call sites updated to pass
both. test_mine_queue passes unchanged.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jphein added a commit that referenced this pull request May 29, 2026
#188)

Fifth and final write surface for #179. The watcher isn't an HTTP
endpoint so it can't take a pydantic body — but the same architectural
goal applies: canonicalize wing at the *input* boundary so all
downstream code can trust the WatchTarget is canonical.

Pre-#179: parse_watch_dirs ran ``normalize_wing_name`` only on
path-derived wings (when env entry was bare ``path`` with no
``=wing``). Explicit ``path=Wing_Name`` env entries were passed through
verbatim, and a defensive ``_normalize_wing_slug`` ran later in
_internal_mine before subprocess spawn.

Post-#179: parse_watch_dirs normalizes both branches (path-derived AND
explicit) through normalize_wing_name. WatchTargets always carry
canonical slugs. The use-time normalize in _internal_mine drops to a
comment explaining the new invariant.

Test added: ``Palace_Daemon`` and ``palace_daemon`` env entries
produce identical WatchTarget.wing values ("palace_daemon"). Was the
shape of the pre-#175/#178 bug class — mixed-case writes paired with
canonical reads producing empty result sets.

This closes the wing-canonicalization migration that began with #172.
All 11 wing/room-accepting sites in palace-daemon now canonicalize at
the input boundary:

Read side (#180):
  GET /search, GET /list, GET /search/fast — FastAPI Depends()
Write body (#181, #182, #183, #184, #186, #187):
  POST /search/keyword, POST /search/hybrid, POST /search/age-fused,
  POST /backfill-age, POST /silent-save, POST /mine, POST /memory —
  pydantic body models in search_models.py
Internal env (this PR):
  PALACE_WATCH_DIRS via watcher.parse_watch_dirs

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants