Skip to content

feat(#179): pydantic body model for /mine (3rd write surface) - #184

Merged
jphein merged 1 commit into
mainfrom
feat/mine-pydantic-body
May 29, 2026
Merged

feat(#179): pydantic body model for /mine (3rd write surface)#184
jphein merged 1 commit into
mainfrom
feat/mine-pydantic-body

Conversation

@jphein

@jphein jphein commented May 29, 2026

Copy link
Copy Markdown
Collaborator

Third write-surface migration after #182 (BackfillAgeBody) and #183 (SilentSaveBody). /mine has the largest body and most elaborate validation of the write endpoints.

Adds MineBody with:

The request param is preserved for request.app.state.active_mines access (subprocess tracking from #138/#139). Test fixture renamed _request → _request_and_body returning (Request, MineBody) tuple.

2 write surfaces remain: POST /memory, watcher auto-mine.

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>
Copilot AI review requested due to automatic review settings May 29, 2026 01:25
@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 e017821 into main May 29, 2026
1 check failed
@jphein
jphein deleted the feat/mine-pydantic-body branch May 29, 2026 01:26

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
Fourth write-surface #179 migration. /memory is the primary write
endpoint and has the most elaborate room-error contract (structured
detail with valid_rooms + hint).

Adds MemoryBody to search_models.py:
  - content: optional, defaults "" (preserves pre-#179 permissive
    behavior — body.get("content", ""))
  - wing: empty → "unknown" then normalize_wing_slug (matches the
    inline default)
  - room: empty → "discoveries" (spec's catch-all), then validated
    via the shared rooms.validate_room_or_raise which raises 400
    with the existing structured detail (error / valid_rooms / hint)

Empty-wing semantics per write surface now form a complete matrix
established across #179 / #182 / #183 / #184 / this PR:

  /memory       → "unknown"        (this PR)
  /silent-save  → ""  + warning    (#183 SilentSaveBody)
  /mine         → "general"        (#184 MineBody)
  /backfill-age → None (filter)    (#182 BackfillAgeBody)

The signature change removes the `request: Request` parameter since
/memory has no need for request.app.state — the only other write
surface keeping Request is /mine, which uses app.state.active_mines
for subprocess tracking (#138/#139).

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