Skip to content

feat(#179 Option A): FastAPI dependencies for wing/room canonicalization (read query-params) - #180

Merged
jphein merged 1 commit into
mainfrom
feat/canonicalization-deps
May 29, 2026
Merged

feat(#179 Option A): FastAPI dependencies for wing/room canonicalization (read query-params)#180
jphein merged 1 commit into
mainfrom
feat/canonicalization-deps

Conversation

@jphein

@jphein jphein commented May 29, 2026

Copy link
Copy Markdown
Collaborator

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.

Two dep factories in rooms.py:

  • rooms.wing_filter_dep — wraps normalize_wing_filter
  • rooms.room_validator_dep — wraps validate_room_or_raise

Migrated:

  • GET /search (wing + room)
  • GET /list (wing + room)
  • GET /search/fast (wing only)

POST endpoints stay on inline calls — they parse JSON bodies and need pydantic models for full Depends integration. Filed as follow-up scope in #179.

Tests: 531 → 534.

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>
Copilot AI review requested due to automatic review settings May 29, 2026 00:57
@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 e37bb6f into main May 29, 2026
1 check failed
@jphein
jphein deleted the feat/canonicalization-deps branch May 29, 2026 00:57

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
#181)

Closes the second half of #179. After PR #180 moved query-param read
endpoints to FastAPI dependencies, the three POST body endpoints
(/search/hybrid, /search/keyword, /search/age-fused) were still using
inline body.get(...) + validate/normalize calls.

Add three pydantic models in search_models.py:
  - SearchKeywordBody  (4 fields)
  - SearchHybridBody   (8 fields incl. fusion_mode/candidate_strategy)
  - SearchAgeFusedBody (7 fields incl. graph_top_k/fusion_k)

Each model has field validators that route:
  - wing → rooms.normalize_wing_filter
  - room → rooms.validate_room_or_raise
  - query → strip+nonempty
  - fusion_mode → enum check (hybrid only)

Refactor each endpoint signature from
  `async def X(request: Request, x_api_key=...)` →
  `async def X(body: TheModel, x_api_key=...)`

Handler bodies now read body.query / body.wing / etc. — no inline
parsing or validation needed. Pydantic ValidationError maps to HTTP
422 (was previously inline HTTP 400 for explicit checks).

Test updates:
  - test_search_hybrid_fusion_mode: construct SearchHybridBody from
    dict; expected status for invalid fusion_mode 400 → 422
  - test_search_age_fused_hydration: construct SearchAgeFusedBody
    from dict

All 534 tests pass. Together with PR #180, this closes the structural
enforcement gap from #179: every wing/room-accepting endpoint now
canonicalizes at the request-parse layer, not at the handler-body
call-site.

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

After #180 + #181 migrated all 6 read endpoints to dependency/pydantic
canonicalization, /backfill-age is the simplest write surface — 4
body fields, admin-only, low-traffic. Apply the same pattern as a
template for the remaining 4 write surfaces.

Adds BackfillAgeBody to search_models.py with field_validator routing
wing through rooms.normalize_wing_filter. All fields optional with
safe defaults so the endpoint still accepts an empty POST body
(curl -X POST .../backfill-age with no Content-Type).

Refactor signature from
  `async def backfill_age(request, x_api_key)` →
  `async def backfill_age(request, body: BackfillAgeBody = Body(default_factory=...), x_api_key)`

Handler body reads body.wing / body.skip_palace / etc. instead of
body.get("wing") / .get("skip_palace"). The `request` parameter is
preserved because the lifespan-cleanup code reads
request.app.state.active_mines (subprocess tracking from #138/#139).

Tests pass (534). No behavior change beyond moving the wing
canonicalization from handler-body to parse-time.

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
#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>
jphein added a commit that referenced this pull request May 29, 2026
…s incident

#179 closed — wing/room canonicalization now structural across all 11
surfaces (#180#188). Documents the #187 validate_default regression and
the #185 stale-deploy incident (dead Syncthing on familiar let deploy.sh
restart on stale source while reporting success).

Reorders open work: #185 promoted to "actionable, unblocked" as the
smallest next item.

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