diff --git a/HANDOFF.md b/HANDOFF.md index 833a954..8ba0d40 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -208,9 +208,10 @@ Each PR ships independently green (`ruff` + `mypy minx_mcp` + `pytest`), with mi - One spec doc under `docs/superpowers/specs/YYYY-MM-DD-slice6X-*.md` (adversarially reviewed before implementation). - One plan doc under `docs/superpowers/plans/YYYY-MM-DD-slice6X-*.md` (step-by-step execution checklist). -- Sequentially numbered migration (next filename: `025_*.sql`). Slice 9 investigations should claim the next available migration. +- Sequentially numbered migration (next filename: `027_*.sql` after `026_memory_capture_fts.sql`). Slice 9 investigations should claim the next available migration. - Implemented-slices row appended to the table above, with LOC / date / verification block. - Operator post-upgrade step added under "Post-Upgrade Operator Steps" if the migration is not fully reversible from application-level data (6g needed a backfill; 6i will need a one-shot FTS5 rebuild; 6l will need a one-shot embedding backfill and a cost-ceiling env var). +- After deploying `026_memory_capture_fts.sql`, run `python -m scripts.rebuild_memory_fts /path/to/minx.db` so any pre-existing `captured_thought` rows are indexed by `payload.text` and `payload.capture_type`. ### Open questions (to resolve in each slice's spec pass) diff --git a/README.md b/README.md index c9a87fe..127f4fe 100644 --- a/README.md +++ b/README.md @@ -148,6 +148,12 @@ Set the `core/llm_config` preference to a payload like: Memory embeddings are offline-safe by default. `memory_hybrid_search` always works through SQLite FTS5; it reranks FTS candidates with stored embeddings only when OpenRouter is configured and compatible candidate embeddings exist. +## Quick Capture Vs Structured Create + +Use `memory_capture` for fast, review-first notes. It stores `captured_thought` memories with default `confidence=0.5`, so rows are candidates until `memory_confirm` promotes them. Use `memory_create` when the caller already has a structured memory payload and intentionally wants the normal confidence/status behavior, including active rows at high confidence. + +`memory_search` defaults to `status="active"`, so reviewers looking for captures should pass `status="candidate"` or `status=null`. Capture acknowledgements expose `response_template` / `response_slots`; Hermes or another harness owns the final user-facing wording. + Set `MINX_OPENROUTER_API_KEY` to enable `memory_embedding_enqueue` and `enrichment_sweep` processing for `memory.embedding` jobs. Optional knobs are `MINX_EMBEDDING_MODEL` (default `openai/text-embedding-3-small`), `MINX_EMBEDDING_DIMENSIONS`, `MINX_EMBEDDING_REQUEST_TIMEOUT_S`, and `MINX_EMBEDDING_MAX_COST_MICROUSD`. API keys are read from the environment only and are not returned in MCP responses. For existing databases, run `python -m scripts.rebuild_memory_fts` after pulling Slice 6i and `python -m scripts.backfill_memory_fingerprints` for rows that pre-date Slice 6g fingerprints. diff --git a/docs/superpowers/plans/2026-04-27-generic-memory-capture.md b/docs/superpowers/plans/2026-04-27-generic-memory-capture.md index 9a99f91..a8b820d 100644 --- a/docs/superpowers/plans/2026-04-27-generic-memory-capture.md +++ b/docs/superpowers/plans/2026-04-27-generic-memory-capture.md @@ -51,6 +51,8 @@ COALESCE(json_extract(new.payload_json, '$.text'), '') || ' ' || COALESCE(json_extract(new.payload_json, '$.capture_type'), '') ``` +This key-based extraction intentionally applies to any future memory type that stores canonical `payload.text` or `payload.capture_type`, not only `captured_thought`. + Use `new.payload_json` in `INSERT` trigger and `new.payload_json` in `UPDATE` trigger (mirror `025` exactly). Header comment should mention capture FTS and that existing DBs should run `python -m scripts.rebuild_memory_fts ` after upgrade. - [ ] **Step 2: Sanity check migration is picked up** @@ -65,12 +67,9 @@ Run: `python -c "from pathlib import Path; import tempfile; from minx_mcp.db imp Expected: no exception (all migrations including `026` apply). -- [ ] **Step 4: Commit** +- [ ] **Step 4: Review checkpoint** -```bash -git add minx_mcp/schema/migrations/026_memory_capture_fts.sql -git commit -m "feat(memory): migration 026 extends FTS payload_text for capture fields" -``` +Pause for review. Do not commit unless the user explicitly asks for a commit in the current session. --- @@ -123,12 +122,9 @@ Run: `pytest tests/test_rebuild_memory_fts.py::test_rebuild_memory_fts_indexes_c Expected: **PASS** -- [ ] **Step 5: Commit** +- [ ] **Step 5: Review checkpoint** -```bash -git add scripts/rebuild_memory_fts.py tests/test_rebuild_memory_fts.py -git commit -m "feat(memory): rebuild FTS includes captured_thought text and capture_type" -``` +Pause for review. Do not commit unless the user explicitly asks for a commit in the current session. --- @@ -299,12 +295,9 @@ Run: `pytest tests/test_memory_service.py -k "normalize_capture_type or derive_c Expected: **PASS** -- [ ] **Step 5: Commit** +- [ ] **Step 5: Review checkpoint** -```bash -git add minx_mcp/core/memory_capture.py tests/test_memory_service.py -git commit -m "feat(memory): deterministic capture normalization and metadata validation" -``` +Pause for review. Do not commit unless the user explicitly asks for a commit in the current session. --- @@ -472,12 +465,9 @@ Run: `pytest tests/test_core_memory_tools.py -k "memory_capture" -v` Expected: **PASS** -- [ ] **Step 5: Commit** +- [ ] **Step 5: Review checkpoint** -```bash -git add minx_mcp/core/tools/memory.py tests/test_core_memory_tools.py -git commit -m "feat(mcp): add memory_capture tool for captured_thought" -``` +Pause for review. Do not commit unless the user explicitly asks for a commit in the current session. --- @@ -518,12 +508,9 @@ Run: `pytest tests/test_memory_service.py::test_captured_thought_round_trip_list Expected: **PASS** (FTS may already index via triggers once migration exists). -- [ ] **Step 4: Commit** +- [ ] **Step 4: Review checkpoint** -```bash -git add minx_mcp/core/memory_payloads.py tests/test_memory_service.py -git commit -m "docs(memory): note captured_thought uses permissive payload validation" -``` +Pause for review. Do not commit unless the user explicitly asks for a commit in the current session. --- @@ -562,12 +549,9 @@ Add **"Quick capture vs structured create"** under the README memory section nea - After deploy, run migrations (automatic on `get_connection` fresh apply; existing servers need app restart / migrate path per your ops). - Run `python -m scripts.rebuild_memory_fts /path/to/minx.db` so pre-existing `captured_thought` rows pick up new FTS columns. -- [ ] **Step 3: Commit** +- [ ] **Step 3: Review checkpoint** -```bash -git add README.md HANDOFF.md -git commit -m "docs: document memory_capture and FTS rebuild for rollout" -``` +Pause for review. Do not commit unless the user explicitly asks for a commit in the current session. --- diff --git a/docs/superpowers/specs/2026-04-19-slice9-agentic-investigations.md b/docs/superpowers/specs/2026-04-19-slice9-agentic-investigations.md index f15d92c..8a18eac 100644 --- a/docs/superpowers/specs/2026-04-19-slice9-agentic-investigations.md +++ b/docs/superpowers/specs/2026-04-19-slice9-agentic-investigations.md @@ -10,14 +10,16 @@ Give Minx the ability to answer **open-ended questions with unknown tool sequenc ## 2) Scope Boundary — Why This Is Not Slice 8 -| Property | Slice 8 Playbooks | Slice 9 Investigations | -|---|---|---| -| Trigger | Cron / event | User-initiated (one-off) | -| Tool sequence | Pre-scripted | Chosen by LLM at each step | -| Cost profile | Bounded, predictable (N calls) | Variable, needs per-run budget | -| Audit shape | `playbook_runs` row | `investigations` row with trajectory | -| Failure mode | Crash mid-script | Agent loops / goes off-rails | -| Output | Side effects (vault writes, logs) | An answer (optionally persisted) | + +| Property | Slice 8 Playbooks | Slice 9 Investigations | +| ------------- | --------------------------------- | ------------------------------------ | +| Trigger | Cron / event | User-initiated (one-off) | +| Tool sequence | Pre-scripted | Chosen by LLM at each step | +| Cost profile | Bounded, predictable (N calls) | Variable, needs per-run budget | +| Audit shape | `playbook_runs` row | `investigations` row with trajectory | +| Failure mode | Crash mid-script | Agent loops / goes off-rails | +| Output | Side effects (vault writes, logs) | An answer (optionally persisted) | + **Rule of thumb:** recurring + predictable → playbook. One-shot + unpredictable → investigation. Recurring + unpredictable is a design smell; split it into a scheduled trigger that fires an investigation. @@ -26,6 +28,7 @@ Give Minx the ability to answer **open-ended questions with unknown tool sequenc **Harness-side.** Core stays a toolbox. Reasons: + 1. **LLM binding is already harness-side** — Core exposes data and templates; agent loops are just more LLM calls, chosen by the LLM. 2. **Cost/killability is a harness concern** — Hermes sets per-invocation budgets (`max_tool_calls`, `max_tokens`, wall-clock timeout). Core shouldn't know about that. 3. **Trace viewing belongs next to the UI** — users asking "why did Minx do that?" want to scrub a trajectory; that's Hermes' job. @@ -33,19 +36,21 @@ Reasons: ### What Core contributes -- **Durable storage** for investigation records (question + answer + trajectory + cost). -- **One log tool** (`log_investigation`) so the harness can persist a run. +- **Durable storage** for investigation records (question + harness-authored answer + trajectory + cost + latest render event). +- **Lifecycle logging tools** (`start_investigation`, `append_investigation_step`, `complete_investigation`) plus a convenience wrapper (`log_investigation`) so the harness can persist a run. - **Retrieval** (`investigation_history`, `investigation_get`) so users and the LLM can reference past investigations. -- **Nothing new for the tool surface** — every `finance_*`, `memory_*`, `goal_*`, `get_insight_history`, `meals_*`, `training_*` tool already in place is exactly what the agent loop picks from. +- **No new domain tools for the agent loop** — every `finance_`*, `memory_*`, `goal_*`, `get_insight_history`, `meals_*`, `training_*` tool already in place is exactly what the agent loop picks from. Slice 9 only adds investigation lifecycle/history tools for audit and retrieval. ## 4) Example Surfaces (Harness-side) -| Surface | Why agentic | Indicative trajectory | -|---|---|---| -| `minx_investigate(question)` | Causal/exploratory questions. LLM decides whether to drill into merchants, categories, meals, goals. | `finance_categories` → `finance_transactions(...)` → maybe `meals_list` → maybe `get_insight_history` → compose | -| `minx_plan(objective)` | Scheduling/planning across domains. Depends on what it finds. | `goal_list` → `get_goal_trajectory` → `training_list` → `meals_list` → draft → revise | -| `minx_retro(period, subject)` | Causal analysis across months. LLM picks which detectors to replay, which transactions to sample. | `get_insight_history` → `goal_trajectory` → sampling tools → synthesize | -| `minx_onboard_entity(kind, name)` | Hydrates an entity/pattern page from scratch. Branches on what it finds. | `finance_transactions(merchant=...)` → `memory_list(subject=...)` → maybe `persist_note` | + +| Surface | Why agentic | Indicative trajectory | +| --------------------------------- | ---------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------- | +| `minx_investigate(question)` | Causal/exploratory questions. LLM decides whether to drill into merchants, categories, meals, goals. | `finance_categories` → `finance_transactions(...)` → maybe `meals_list` → maybe `get_insight_history` → compose | +| `minx_plan(objective)` | Scheduling/planning across domains. Depends on what it finds. | `goal_list` → `get_goal_trajectory` → `training_list` → `meals_list` → draft → revise | +| `minx_retro(period, subject)` | Causal analysis across months. LLM picks which detectors to replay, which transactions to sample. | `get_insight_history` → `goal_trajectory` → sampling tools → synthesize | +| `minx_onboard_entity(kind, name)` | Hydrates an entity/pattern page from scratch. Branches on what it finds. | `finance_transactions(merchant=...)` → `memory_list(subject=...)` → maybe `persist_note` | + Common shape: **one question in, one report out, unpredictable middle.** @@ -63,8 +68,11 @@ CREATE TABLE investigations ( context_json TEXT, -- structured inputs (date range, subject, etc.) status TEXT NOT NULL -- 'running' | 'succeeded' | 'failed' | 'cancelled' | 'budget_exhausted' CHECK (status IN ('running', 'succeeded', 'failed', 'cancelled', 'budget_exhausted')), - answer_md TEXT, -- final rendered answer (markdown) - trajectory_json TEXT, -- [{step, tool, args, result_digest, latency_ms}, ...] + answer_md TEXT, -- optional harness-authored rendered answer (markdown) + trajectory_json TEXT, -- [{step, event_template, event_slots, tool, args_digest, result_digest, latency_ms}, ...] + response_template TEXT, -- latest lifecycle render event, e.g. investigation.completed + response_slots_json TEXT, -- JSON slots for latest lifecycle render event + citation_refs_json TEXT, -- references used by the harness answer tool_call_count INTEGER, token_input INTEGER, token_output INTEGER, @@ -80,11 +88,17 @@ CREATE INDEX idx_investigations_running ON investigations(status) WHERE status = **Trajectory storage policy:** `trajectory_json` stores a **digest** per step (tool name, arg hash, result row count / bytes, latency). It does NOT store full tool outputs — those can be large and contain PII. Full outputs are reconstructable by replaying the tools against the DB at investigation time. +**Render storage policy:** `response_template` and `response_slots_json` store the latest lifecycle event so read APIs can expose a stable render surface without parsing trajectory text. Step-level render events are stored inside `trajectory_json` step entries. + ## 6) Core MCP Tools +Lifecycle responses follow the render-contract amendment in `2026-04-28-slice9-investigation-render-contract.md`: tools return the ids below plus `response_template` / `response_slots` for lifecycle transitions. The minimal shapes shown here are the base data fields, not the complete MCP response contract. + ``` -start_investigation(kind, question, context_json, harness) -> {"investigation_id": int} -append_investigation_step(investigation_id, step_json) -> {"ok": true} +start_investigation(kind, question, context_json, harness) + -> {"investigation_id": int, "response_template": "investigation.started", "response_slots": {...}} +append_investigation_step(investigation_id, step_json) + -> {"ok": true, "response_template": "investigation.step_logged|investigation.needs_confirmation", "response_slots": {...}} complete_investigation( investigation_id, status, # 'succeeded' | 'failed' | 'cancelled' | 'budget_exhausted' @@ -94,11 +108,12 @@ complete_investigation( token_output, cost_usd, error_message, -) -> {"investigation_id": int} -log_investigation(...) # convenience wrapper, same shape as log_playbook_run +) -> {"investigation_id": int, "response_template": "investigation.completed|investigation.failed|investigation.cancelled|investigation.budget_exhausted", "response_slots": {...}} +log_investigation(...) # convenience wrapper with the same logging role as log_playbook_run; + # MCP return shape follows the render-contract amendment investigation_history(kind=None, since=None, days=30, limit=100) -> {"runs": [...], "truncated": bool} -investigation_get(investigation_id) -> {"run": {...}} # includes trajectory +investigation_get(investigation_id) -> {"run": {...}} # includes trajectory and latest response_template/response_slots ``` Mirrors the two-phase + convenience pattern from Slice 8 so the audit story is consistent. @@ -140,15 +155,17 @@ except Exception as exc: Split so Core can ship independently and the harness can build against a stable surface. -| Phase | What | Where | Effort | Dependencies | -|---|---|---|---|---| -| 9a | `investigations` table (next available migration) + `start_/append_/complete_/log_investigation` + `investigation_history` + `investigation_get` + tests | Core | 1.5 days | Slice 6i-6l + Slice 8a merged | -| 9b | Trajectory-digest helpers (tool name + arg hash + result digest) + PII-redaction pass for `context_json`/`answer_md` + operator runbook | Core | 1 day | 9a | -| 9c | Investigation MCP resource surface: `investigation://recent`, `investigation://{id}` (read-only) for harness UIs that want to render history without hitting the tool API | Core | 0.5 day | 9a | -| 9d | Reference harness loop (Hermes) for `minx_investigate` — budget wrapper, LLM tool-picker, digest-and-log loop | Hermes | 3-4 days | 9a, 9b | -| 9e | `minx_plan` surface (second agentic entry point reusing 9d infra) | Hermes | 2 days | 9d | -| 9f | `minx_retro` + `minx_onboard_entity` surfaces | Hermes | 2-3 days | 9d | -| 9g | Investigation re-query: `memory_list` gains an optional `include_cited_investigations` flag so answers can reference prior investigations | Core | 1 day | 9a, Slice 6 | + +| Phase | What | Where | Effort | Dependencies | +| ----- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | -------- | ----------------------------- | +| 9a | `investigations` table (next available migration) + `start_/append_/complete_/log_investigation` + `investigation_history` + `investigation_get` + tests | Core | 1.5 days | Slice 6i-6l + Slice 8a merged | +| 9b | Trajectory-digest helpers (tool name + arg hash + result digest) + PII-redaction pass for `context_json`/`answer_md` + operator runbook | Core | 1 day | 9a | +| 9c | Investigation MCP resource surface: `investigation://recent`, `investigation://{id}` (read-only) for harness UIs that want to render history without hitting the tool API | Core | 0.5 day | 9a | +| 9d | Reference harness loop (Hermes) for `minx_investigate` — budget wrapper, LLM tool-picker, digest-and-log loop | Hermes | 3-4 days | 9a, 9b | +| 9e | `minx_plan` surface (second agentic entry point reusing 9d infra) | Hermes | 2 days | 9d | +| 9f | `minx_retro` + `minx_onboard_entity` surfaces | Hermes | 2-3 days | 9d | +| 9g | Investigation re-query: `memory_list` gains an optional `include_cited_investigations` flag so answers can reference prior investigations | Core | 1 day | 9a, Slice 6 | + **Core effort: ~4 days (9a + 9b + 9c + 9g).** **Hermes effort: ~7-9 days (9d + 9e + 9f).** @@ -158,6 +175,7 @@ Ship order: 9a → 9b → 9d (first usable surface) → 9c + 9e + 9f + 9g in any ## 10) Testing Strategy ### Core tests (9a–9c) + - Two-phase lifecycle (`start` → `append` × N → `complete`) covering succeeded / failed / cancelled / budget_exhausted. - Concurrent starts with different kinds don't collide; same-kind concurrent is allowed (investigations are user-initiated, no cron contention). - `append_investigation_step` rejects steps after terminal status. @@ -165,6 +183,7 @@ Ship order: 9a → 9b → 9d (first usable surface) → 9c + 9e + 9f + 9g in any - `investigation_history` pagination/filter matches `playbook_history` semantics. ### Harness tests (9d–9f, outside this repo) + - Budget caps respected (max_tool_calls, max_tokens, wall_clock). - Answer is always produced for `budget_exhausted` (partial answer, never a hard crash). - LLM tool-picker respects an allowlist (can't invoke destructive tools like `memory_reject` without explicit surface-level opt-in). @@ -172,5 +191,5 @@ Ship order: 9a → 9b → 9d (first usable surface) → 9c + 9e + 9f + 9g in any ## 11) Relationship to Other Slices - **Slice 6 (Memory):** investigations can cite memories by id in `answer_md`; `memory_get`, `memory_list`, FTS5 search, memory graph edges, and embeddings/hybrid retrieval are primary inputs. -- **Slice 8 (Playbooks):** audit pattern (two-phase + convenience wrapper + reconcile-crashed) is lifted directly. `playbook_reconcile_crashed` gets a sibling `investigation_reconcile_crashed`. -- **Slice 5 (Harness Adaptation):** a second harness would reimplement the loop against the same Core API. The agent-loop pattern is harness-specific; the tool surface is portable. +- **Slice 8 (Playbooks):** audit pattern (two-phase + convenience wrapper) is lifted directly. A sibling to `playbook_reconcile_crashed` can be added later if crashed-running investigations need automated reconciliation; specify that tool explicitly before shipping it. +- **Slice 5 (Harness Adaptation):** a second harness would reimplement the loop against the same Core API. The agent-loop pattern is harness-specific; the tool surface is portable. \ No newline at end of file diff --git a/docs/superpowers/specs/2026-04-27-generic-memory-capture.md b/docs/superpowers/specs/2026-04-27-generic-memory-capture.md index 1d48e63..e548b41 100644 --- a/docs/superpowers/specs/2026-04-27-generic-memory-capture.md +++ b/docs/superpowers/specs/2026-04-27-generic-memory-capture.md @@ -117,6 +117,8 @@ COALESCE(json_extract(payload_json, '$.capture_type'), '') Do not blindly flatten arbitrary `metadata` into FTS. Searches over metadata can wait for a dedicated design. +Adding `$.text` and `$.capture_type` to the generic memory FTS extraction means any future memory type that uses those canonical payload keys will also be indexed. That is intentional; the extraction is key-based, not limited to `captured_thought`. + Existing databases: operators run `python -m scripts.rebuild_memory_fts` after migration so historical `captured_thought` rows, if any exist, pick up the new extraction. ## Vault / Obsidian diff --git a/docs/superpowers/specs/2026-04-28-goal-parse-render-contract.md b/docs/superpowers/specs/2026-04-28-goal-parse-render-contract.md index 2166b84..467c859 100644 --- a/docs/superpowers/specs/2026-04-28-goal-parse-render-contract.md +++ b/docs/superpowers/specs/2026-04-28-goal-parse-render-contract.md @@ -63,6 +63,9 @@ For clarification outcomes: "options": [ { "kind": "goal", "goal_id": 1, "label": "Dining Out under $250" } ], + "resume_payload": { + "target_value": 25000 + }, "question": "Which goal do you mean?" } ``` @@ -98,10 +101,11 @@ Slots should include only structured data: - `status` - `field` - `candidate_count` -- `payload` Do not place complete user-facing sentences in slots. +Do not duplicate the full top-level `payload` in `response_slots` by default. The top-level `payload` remains the authoritative machine proposal; slots should be a small projection of the normalized values Hermes needs to choose a template rendering, not a second copy of the proposal. If a future template truly needs a preview of a larger proposal, add an explicit small field such as `payload_summary` instead of copying the full payload object. + ## Implementation Shape Likely touched files: @@ -123,9 +127,35 @@ Recommended model changes: Validation: -- `create`, `update`, and `no_match` require `response_template` and `response_slots`. -- `clarify` requires `clarification_template` and `clarification_slots`. -- Existing `assistant_message`, `question`, and `options` validation remains for compatibility unless a later breaking cleanup removes them. +- This migration is additive. Existing compatibility fields remain required where current validation requires them: `assistant_message` for `create`, `update`, and `no_match`; `question` for `clarify`. +- `create`, `update`, and `no_match` require `response_template` and `response_slots`, and must omit `clarification_template` and `clarification_slots`. +- `clarify` requires `clarification_template` and `clarification_slots`, and must omit `response_template` and `response_slots`. +- Existing `options` and `resume_payload` validation remains unchanged: `ambiguous_goal` and `ambiguous_subject` require both non-empty `options` and `resume_payload`; `missing_goal` omits `options`. + +Allowed render fields by result type: + +| `result_type` | Required render fields | Must omit | Compatibility fields | +|---|---|---|---| +| `create` | `response_template`, `response_slots` | `clarification_template`, `clarification_slots` | `assistant_message` | +| `update` | `response_template`, `response_slots` | `clarification_template`, `clarification_slots` | `assistant_message` | +| `no_match` | `response_template`, `response_slots` | `clarification_template`, `clarification_slots` | `assistant_message` | +| `clarify` | `clarification_template`, `clarification_slots` | `response_template`, `response_slots` | `question`, subtype-specific `options` / `resume_payload` | + +Clarification subtype compatibility rules: + +| `clarification_type` | `options` | `resume_payload` | +|---|---|---| +| `ambiguous_goal` | required, non-empty | required | +| `ambiguous_subject` | required, non-empty | required | +| `missing_goal` | omitted | optional continuation context | +| `missing_target` | omitted | optional continuation context | +| `vague_intent` | omitted | optional, usually omitted | + +Serialization: + +- Extend `minx_mcp/core/tools/goals.py::_goal_parse_result_to_dict` to include the new template and slot fields. +- Keep existing serialized fields (`assistant_message`, `question`, `options`, `resume_payload`) while old clients migrate. +- Ensure preferred render fields are emitted for every result, even when the compatibility field has the same semantic outcome. ## LLM Boundary @@ -141,6 +171,7 @@ Add or update tests to cover: - update result includes `response_template == "goal_parse.update.ready"` - no-match result includes `response_template == "goal_parse.no_match.unsupported"` - clarify result includes `clarification_template` based on `clarification_type` +- ambiguous goal/subject clarify results preserve `resume_payload` and options - slots contain structured payload values needed by Hermes - compatibility fields still exist and are deterministic - model-authored prose from an LLM stub is not exposed as preferred render data diff --git a/docs/superpowers/specs/2026-04-28-mcp-render-contract.md b/docs/superpowers/specs/2026-04-28-mcp-render-contract.md index 6cca74a..c2e6484 100644 --- a/docs/superpowers/specs/2026-04-28-mcp-render-contract.md +++ b/docs/superpowers/specs/2026-04-28-mcp-render-contract.md @@ -79,9 +79,12 @@ Template keys should be stable, namespaced, and event-like: - `goal_parse.clarify.ambiguous_goal` - `memory_capture.created_candidate` - `investigation.started` +- `investigation.step_logged` - `investigation.needs_confirmation` - `investigation.completed` - `investigation.failed` +- `investigation.cancelled` +- `investigation.budget_exhausted` Template keys are contracts. Avoid changing them casually; add new keys when behavior meaningfully changes. @@ -107,6 +110,8 @@ class RenderHint: } ``` +Clarification responses should call this helper with `prefix="clarification"` or construct the `clarification_template` / `clarification_slots` fields directly. Do not emit `response_template` for clarification-only outcomes. + Expected homes, in order of preference: 1. `minx_mcp/contracts.py` if multiple MCP packages use it. @@ -144,8 +149,8 @@ For every conversational MCP response: - Assert template fields exist. - Assert slots contain the expected structured values. -- Assert model-authored wording from test LLM payloads is not surfaced in template fields, slots, or fallback strings. -- Assert old compatibility fields remain deterministic when kept. +- Assert model-authored wording from test LLM payloads is not surfaced in template fields or slots. +- Assert old compatibility fields remain deterministic when kept; if fallback strings remain, they may contain deterministic Core wording but must not pass through model-authored prose. For pure data tools: diff --git a/docs/superpowers/specs/2026-04-28-slice9-investigation-render-contract.md b/docs/superpowers/specs/2026-04-28-slice9-investigation-render-contract.md index 8060a0d..57fde82 100644 --- a/docs/superpowers/specs/2026-04-28-slice9-investigation-render-contract.md +++ b/docs/superpowers/specs/2026-04-28-slice9-investigation-render-contract.md @@ -85,10 +85,11 @@ Hermes may render those as "Investigation complete" or a richer explanation, but The existing Slice 9 spec stores `answer_md`. Under this update: - `answer_md` is optional harness-authored output, not Core-authored prose. +- Core may store `answer_md` when Hermes passes it to `complete_investigation`, but Core must not generate or rewrite that final explanation. - Add or reserve `answer_template` and `answer_slots_json` only if Core needs to store a structured final render hint. - Prefer storing citations and structured answer metadata separately from final text when possible. -Recommended additions: +Default implementation additions: ```sql response_template TEXT, @@ -102,11 +103,11 @@ Where: - `response_slots_json` stores JSON slots for that latest event. - `citation_refs_json` stores references such as memory ids, investigation ids, tool result digests, or vault paths used by the harness answer. -If this feels redundant with trajectory storage during implementation, keep event templates in `trajectory_json` step entries instead of adding columns. The key invariant is that template keys/slots must be available to the harness without parsing prose. +Use these columns for the initial Core implementation so `investigation_history` and `investigation_get` have a stable latest-event surface. Step-level events still live in `trajectory_json` entries. A trajectory-only storage approach should be a deliberate later simplification, and only if history/get tools continue exposing `response_template`, `response_slots`, and step event fields without parsing prose. ## Trajectory Step Shape -Each appended step should include structured event data: +`append_investigation_step` accepts a single `step_json` object with this shape. Core should validate the required fields and reject raw tool output. Optional extra scalar metadata is acceptable only when JSON-safe and non-sensitive. ```json { @@ -125,7 +126,9 @@ Each appended step should include structured event data: } ``` -No raw tool output should be stored in `event_slots`. +Required fields: `step`, `event_template`, `event_slots`, `tool`, `args_digest`, `result_digest`, and `latency_ms`. + +Allowed `event_slots` values are structured digests, counts, enum-like labels, ids, booleans, numbers, and short normalized strings. No raw tool output should be stored in `event_slots`; use `result_digest`, `row_count`, `byte_count`, or citation ids instead. ## Confirmations @@ -143,7 +146,7 @@ If an investigation needs user confirmation before a risky step, Core should sto } ``` -Hermes renders the prompt and records the user's decision by calling the appropriate Core/domain tool. Core should not invent the confirmation wording. +`append_investigation_step` may return `response_template == "investigation.needs_confirmation"` instead of `investigation.step_logged` when the appended step records a proposed risky action that is waiting on the user. Hermes renders the prompt and records the user's decision by calling the appropriate Core/domain tool. Core should not invent the confirmation wording. ## Testing @@ -162,9 +165,34 @@ Hermes tests, outside this repo, should cover: - Hermes composes final explanation prose. - Hermes enforces budgets and records terminal status in Core. +## Read Surfaces + +`investigation_history` should expose the latest lifecycle render event for each run: + +```json +{ + "runs": [ + { + "investigation_id": 42, + "kind": "investigate", + "status": "succeeded", + "response_template": "investigation.completed", + "response_slots": { + "investigation_id": 42, + "kind": "investigate", + "status": "succeeded" + } + } + ], + "truncated": false +} +``` + +`investigation_get` should include the same latest `response_template` / `response_slots` plus the trajectory entries with each step's `event_template` / `event_slots`. `log_investigation` is a convenience wrapper over the same lifecycle storage rules; it should return the terminal lifecycle render hint that matches the logged status. + ## Relationship To The Existing Slice 9 Spec -This spec amends, rather than replaces, `2026-04-19-slice9-agentic-investigations.md`. +This spec amends, rather than replaces, `2026-04-19-slice9-agentic-investigations.md`. It supersedes that spec's minimal lifecycle response examples for `start_investigation`, `append_investigation_step`, `complete_investigation`, and `log_investigation`: those tools should return the relevant ids plus `response_template` / `response_slots` when they create or transition lifecycle state. When implementing Slice 9: @@ -187,4 +215,4 @@ When implementing Slice 9: - Core owns investigation persistence and auditability. - Hermes owns LLM loop and final prose. - Template keys cover started, confirmation, terminal, and step events. -- The design avoids over-storing raw tool output or raw conversation text. +- The design avoids over-storing raw tool output or raw conversation text. \ No newline at end of file diff --git a/minx_mcp/core/goal_models.py b/minx_mcp/core/goal_models.py index 97bfa4c..a1a0c8c 100644 --- a/minx_mcp/core/goal_models.py +++ b/minx_mcp/core/goal_models.py @@ -159,6 +159,10 @@ class GoalCaptureResult: question: str | None = None options: list[GoalCaptureOption] | None = None resume_payload: dict[str, object] | None = None + response_template: str | None = None + response_slots: dict[str, object] | None = None + clarification_template: str | None = None + clarification_slots: dict[str, object] | None = None def __post_init__(self) -> None: if self.result_type == "create": @@ -174,7 +178,10 @@ def __post_init__(self) -> None: "question", "options", "resume_payload", + "clarification_template", + "clarification_slots", ) + self._populate_response_render_fields() elif self.result_type == "update": if self.action != "goal_update": raise ValueError("action must be goal_update for update results") @@ -189,7 +196,10 @@ def __post_init__(self) -> None: "question", "options", "resume_payload", + "clarification_template", + "clarification_slots", ) + self._populate_response_render_fields() elif self.result_type == "clarify": if self.clarification_type is None: raise ValueError("clarification_type is required for clarify results") @@ -224,6 +234,8 @@ def __post_init__(self) -> None: if self.clarification_type == "missing_goal" and self.options is not None: raise ValueError("options must be omitted for missing_goal clarify results") self._require_absent("payload", "goal_id", "assistant_message") + self._require_absent("response_template", "response_slots") + self._populate_clarification_render_fields() elif self.result_type == "no_match": if self.assistant_message is None: raise ValueError("assistant_message is required for no_match results") @@ -235,7 +247,10 @@ def __post_init__(self) -> None: "question", "options", "resume_payload", + "clarification_template", + "clarification_slots", ) + self._populate_response_render_fields() else: raise ValueError("result_type is invalid") @@ -244,6 +259,108 @@ def _require_absent(self, *field_names: str) -> None: if getattr(self, field_name) is not None: raise ValueError(f"{field_name} must be omitted for {self.result_type} results") + def _populate_response_render_fields(self) -> None: + if self.response_template is None: + object.__setattr__(self, "response_template", _response_template_for(self)) + if self.response_slots is None: + object.__setattr__(self, "response_slots", _response_slots_for(self)) + + def _populate_clarification_render_fields(self) -> None: + if self.clarification_template is None: + object.__setattr__( + self, + "clarification_template", + f"goal_parse.clarify.{self.clarification_type}", + ) + if self.clarification_slots is None: + object.__setattr__(self, "clarification_slots", _clarification_slots_for(self)) + + +def _response_template_for(result: GoalCaptureResult) -> str: + if result.result_type == "create": + return "goal_parse.create.ready" + if result.result_type == "update": + return "goal_parse.update.ready" + if result.result_type == "no_match": + return "goal_parse.no_match.unsupported" + raise ValueError("response_template is invalid for clarify results") + + +def _response_slots_for(result: GoalCaptureResult) -> dict[str, object]: + if result.result_type == "create": + payload = result.payload or {} + slots: dict[str, object] = {"action": "goal_create"} + _copy_slot(payload, slots, "goal_type") + subject = _subject_from_payload(payload) + if subject is not None: + slots["subject"] = subject + subject_kind = _subject_kind_from_payload(payload) + if subject_kind is not None: + slots["subject_kind"] = subject_kind + _copy_slot(payload, slots, "period") + _copy_slot(payload, slots, "target_value") + return slots + if result.result_type == "update": + slots = {"action": "goal_update", "goal_id": result.goal_id} + for key in ("status", "target_value", "title"): + _copy_slot(result.payload or {}, slots, key) + return slots + if result.result_type == "no_match": + return {"status": "unsupported"} + raise ValueError("response_slots are invalid for clarify results") + + +def _clarification_slots_for(result: GoalCaptureResult) -> dict[str, object]: + slots: dict[str, object] = {} + if result.action is not None: + slots["action"] = result.action + slots["field"] = _clarification_field(result.clarification_type) + if result.options: + slots["candidate_count"] = len(result.options) + return slots + + +def _clarification_field(clarification_type: GoalCaptureClarificationType | None) -> str: + if clarification_type == "ambiguous_goal": + return "goal_id" + if clarification_type == "ambiguous_subject": + return "subject" + if clarification_type == "missing_goal": + return "goal_id" + if clarification_type == "missing_target": + return "target_value" + if clarification_type == "vague_intent": + return "subject" + raise ValueError("clarification_type is required for clarify results") + + +def _copy_slot(source: dict[str, object], target: dict[str, object], key: str) -> None: + value = source.get(key) + if value is not None: + target[key] = value + + +def _subject_from_payload(payload: dict[str, object]) -> str | None: + for key in ("category_names", "merchant_names", "account_names"): + values = payload.get(key) + if isinstance(values, list) and values: + first = values[0] + return first if isinstance(first, str) else None + title = payload.get("title") + return title if isinstance(title, str) else None + + +def _subject_kind_from_payload(payload: dict[str, object]) -> str | None: + for key, kind in ( + ("category_names", "category"), + ("merchant_names", "merchant"), + ("account_names", "account"), + ): + values = payload.get(key) + if isinstance(values, list) and values: + return kind + return None + @dataclass(frozen=True) class GoalProgress: diff --git a/minx_mcp/core/memory_capture.py b/minx_mcp/core/memory_capture.py new file mode 100644 index 0000000..0f53125 --- /dev/null +++ b/minx_mcp/core/memory_capture.py @@ -0,0 +1,158 @@ +from __future__ import annotations + +import math +import re +from collections.abc import Mapping, Sequence +from typing import TYPE_CHECKING + +from minx_mcp.contracts import InvalidInputError + +if TYPE_CHECKING: + from minx_mcp.core.memory_models import MemoryRecord + +CAPTURE_TYPE_MAX_BYTES = 64 +SUBJECT_MAX_BYTES = 200 +METADATA_MAX_TOP_LEVEL_KEYS = 32 +METADATA_MAX_DEPTH = 4 +METADATA_STRING_MAX_BYTES = 4096 + + +def normalize_capture_text_for_body(text: str) -> str: + return _collapse_whitespace(text.strip()) + + +def normalize_capture_type(raw: str) -> str: + stripped = raw.strip() + if not stripped: + return "observation" + + lowered = "".join(char.lower() if "A" <= char <= "Z" else char for char in stripped) + normalized = re.sub(r"\s+", "_", lowered) + normalized = re.sub(r"[^a-z0-9_-]+", "_", normalized) + normalized = re.sub(r"_+", "_", normalized).strip("_") + if not normalized: + return "observation" + return _truncate_utf8_with_ellipsis(normalized, CAPTURE_TYPE_MAX_BYTES) + + +def derive_capture_subject( + *, + capture_type_normalized: str, + raw_text: str, + explicit_subject: str | None, +) -> str: + if explicit_subject is not None: + subject = explicit_subject.strip() + if not subject: + raise InvalidInputError("subject must be non-empty") + return _truncate_utf8_with_ellipsis(subject, SUBJECT_MAX_BYTES) + + fragment = "capture" + for line in raw_text.splitlines(): + collapsed = _collapse_whitespace(line.strip()) + if collapsed: + fragment = collapsed + break + return _truncate_utf8_with_ellipsis( + f"{capture_type_normalized}:{fragment}", + SUBJECT_MAX_BYTES, + ) + + +def validate_capture_metadata(meta: object) -> dict[str, object] | None: + if meta is None: + return None + if not isinstance(meta, dict): + raise InvalidInputError("metadata must be a JSON object") + if not meta: + return None + if len(meta) > METADATA_MAX_TOP_LEVEL_KEYS: + raise InvalidInputError( + f"metadata must contain at most {METADATA_MAX_TOP_LEVEL_KEYS} top-level keys" + ) + _validate_metadata_node(meta, depth=1) + return dict(meta) + + +def build_captured_thought_payload( + *, + text: str, + capture_type: str, + metadata: dict[str, object] | None, +) -> dict[str, object]: + payload: dict[str, object] = {"text": text, "capture_type": capture_type} + if metadata is not None: + payload["metadata"] = metadata + return payload + + +def build_capture_response_slots(*, record: MemoryRecord, capture_type: str) -> dict[str, object]: + return { + "memory_id": record.id, + "status": record.status, + "memory_type": record.memory_type, + "scope": record.scope, + "subject": record.subject, + "capture_type": capture_type, + } + + +def _truncate_utf8_with_ellipsis(value: str, max_bytes: int) -> str: + encoded = value.encode("utf-8") + if len(encoded) <= max_bytes: + return value + suffix = "..." + suffix_len = len(suffix.encode("utf-8")) + budget = max_bytes - suffix_len + if budget <= 0: + return suffix[:max_bytes] + + out: list[str] = [] + used = 0 + for char in value: + char_len = len(char.encode("utf-8")) + if used + char_len > budget: + break + out.append(char) + used += char_len + return "".join(out) + suffix + + +def _collapse_whitespace(value: str) -> str: + return re.sub(r"\s+", " ", value) + + +def _validate_metadata_node(value: object, *, depth: int) -> None: + if value is None or isinstance(value, bool): + return + if isinstance(value, int): + return + if isinstance(value, float): + if not math.isfinite(value): + raise InvalidInputError("metadata numeric values must be finite") + return + if isinstance(value, str): + if len(value.encode("utf-8")) > METADATA_STRING_MAX_BYTES: + raise InvalidInputError( + f"metadata string values must be at most {METADATA_STRING_MAX_BYTES} bytes" + ) + return + + if isinstance(value, Mapping): + if depth > METADATA_MAX_DEPTH: + raise InvalidInputError(f"metadata nesting depth must be at most {METADATA_MAX_DEPTH}") + for key, child in value.items(): + if not isinstance(key, str): + raise InvalidInputError("metadata keys must be strings") + _validate_metadata_node(child, depth=depth + 1) + return + + if isinstance(value, Sequence) and not isinstance(value, (bytes, bytearray, str)): + if depth > METADATA_MAX_DEPTH: + raise InvalidInputError(f"metadata nesting depth must be at most {METADATA_MAX_DEPTH}") + for child in value: + _validate_metadata_node(child, depth=depth + 1) + return + + raise InvalidInputError("metadata values must be JSON-compatible") + diff --git a/minx_mcp/core/memory_payloads.py b/minx_mcp/core/memory_payloads.py index fb6a2c7..f74e0f9 100644 --- a/minx_mcp/core/memory_payloads.py +++ b/minx_mcp/core/memory_payloads.py @@ -45,6 +45,8 @@ class ConstraintPayload(_ExtraForbidModel): unit: str | None = None # e.g. "USD/week", "grams/day" +# captured_thought is intentionally omitted so quick captures use permissive +# unknown-type validation until they graduate to a stricter schema. PAYLOAD_MODELS: dict[str, type[_ExtraForbidModel]] = { "preference": PreferencePayload, "pattern": PatternPayload, diff --git a/minx_mcp/core/tools/goals.py b/minx_mcp/core/tools/goals.py index 36cb414..34f53a0 100644 --- a/minx_mcp/core/tools/goals.py +++ b/minx_mcp/core/tools/goals.py @@ -390,10 +390,18 @@ def _goal_parse_result_to_dict(result: GoalCaptureResult) -> dict[str, object]: data["action"] = result.action if result.payload is not None: data["payload"] = result.payload + if result.response_template is not None: + data["response_template"] = result.response_template + if result.response_slots is not None: + data["response_slots"] = result.response_slots if result.goal_id is not None: data["goal_id"] = result.goal_id if result.clarification_type is not None: data["clarification_type"] = result.clarification_type + if result.clarification_template is not None: + data["clarification_template"] = result.clarification_template + if result.clarification_slots is not None: + data["clarification_slots"] = result.clarification_slots if result.question is not None: data["question"] = result.question if result.options is not None: @@ -411,8 +419,10 @@ def _goal_capture_option_to_dict(option: GoalCaptureOption) -> dict[str, object] "payload_fragment": option.payload_fragment, } return { + "kind": option.kind, "goal_id": option.goal_id, "title": option.title, + "label": option.label, "period": option.period, "target_value": option.target_value, "status": option.status, diff --git a/minx_mcp/core/tools/memory.py b/minx_mcp/core/tools/memory.py index 6cb21be..1f1ca31 100644 --- a/minx_mcp/core/tools/memory.py +++ b/minx_mcp/core/tools/memory.py @@ -1,4 +1,4 @@ -"""Memory MCP tools: list / get / create / confirm / reject / expire / candidates.""" +"""Memory MCP tools: list / get / create / capture / confirm / reject / expire / candidates.""" from __future__ import annotations @@ -9,6 +9,14 @@ from minx_mcp.contracts import InvalidInputError, ToolResponse, wrap_tool_call from minx_mcp.core import memory_embeddings from minx_mcp.core.enrichment_queue import EnrichmentJob +from minx_mcp.core.memory_capture import ( + build_capture_response_slots, + build_captured_thought_payload, + derive_capture_subject, + normalize_capture_text_for_body, + normalize_capture_type, + validate_capture_metadata, +) from minx_mcp.core.memory_embeddings import ( enqueue_memory_embedding, hybrid_memory_search, @@ -66,6 +74,39 @@ def memory_create_tool( tool_name="memory_create", ) + @mcp.tool(name="memory_capture") + def memory_capture_tool( + text: str, + capture_type: str = "observation", + scope: str = "core", + subject: str | None = None, + source: str = "user:capture", + confidence: float | int = 0.5, + metadata: object | None = None, + ) -> ToolResponse: + """Quick-capture text as a candidate memory for later review. + + Defaults to capture_type="observation", scope="core", source="user:capture", + and confidence=0.5. Captures must stay below confidence 0.8 and remain + candidate rows until memory_confirm. memory_search defaults to active rows, + so reviewers should pass status="candidate" or status=None to find captures. + Duplicate live captures can return CONFLICT through normal memory dedupe rules. + Harnesses should render acknowledgement copy from response_template/slots. + """ + return wrap_tool_call( + lambda: _memory_capture( + config, + text, + capture_type, + scope, + subject, + source, + confidence, + metadata, + ), + tool_name="memory_capture", + ) + @mcp.tool(name="memory_confirm") def memory_confirm_tool(memory_id: int) -> ToolResponse: return wrap_tool_call( @@ -254,6 +295,60 @@ def _memory_create( return {"memory": memory_record_as_dict(record)} +def _memory_capture( + config: CoreServiceConfig, + text: str, + capture_type: str, + scope: str, + subject: str | None, + source: str, + confidence: float | int, + metadata: object | None, +) -> dict[str, object]: + normalized_text = normalize_capture_text_for_body(text) + if not normalized_text: + raise InvalidInputError("text must be non-empty") + capture_type_normalized = normalize_capture_type(capture_type) + metadata_payload = validate_capture_metadata(metadata) + conf = _coerce_confidence(confidence) + if conf >= 0.8: + raise InvalidInputError( + "confidence must be below 0.8 for memory_capture; use memory_create for active memories" + ) + sc = require_non_empty("scope", scope) + src = require_non_empty("source", source) + sj = derive_capture_subject( + capture_type_normalized=capture_type_normalized, + raw_text=text, + explicit_subject=subject, + ) + payload = build_captured_thought_payload( + text=normalized_text, + capture_type=capture_type_normalized, + metadata=metadata_payload, + ) + with scoped_connection(Path(config.db_path)) as conn: + service = MemoryService(Path(config.db_path), conn=conn) + record = service.create_memory( + memory_type="captured_thought", + scope=sc, + subject=sj, + confidence=conf, + payload=payload, + source=src, + reason="", + actor="user", + ) + return { + "memory": memory_record_as_dict(record), + "response_template": "memory_capture.created_candidate", + "response_slots": build_capture_response_slots( + record=record, + capture_type=capture_type_normalized, + ), + } + + def _memory_confirm(config: CoreServiceConfig, memory_id: int) -> dict[str, object]: mid = _coerce_memory_id(memory_id) with scoped_connection(Path(config.db_path)) as conn: diff --git a/minx_mcp/schema/migrations/026_memory_capture_fts.sql b/minx_mcp/schema/migrations/026_memory_capture_fts.sql new file mode 100644 index 0000000..b0f3ea8 --- /dev/null +++ b/minx_mcp/schema/migrations/026_memory_capture_fts.sql @@ -0,0 +1,59 @@ +-- Slice 6 memory capture: include captured text fields in future FTS trigger writes. +-- +-- Existing FTS rows should be refreshed with scripts/rebuild_memory_fts.py +-- after this migration if the database already contains captured_thought rows. + +DROP TRIGGER IF EXISTS memories_ai_fts; +DROP TRIGGER IF EXISTS memories_au_fts; +DROP TRIGGER IF EXISTS memories_ad_fts; + +CREATE TRIGGER IF NOT EXISTS memories_ai_fts AFTER INSERT ON memories BEGIN + INSERT INTO memory_fts(rowid, memory_type, scope, subject, payload_text, source, reason) + VALUES ( + new.id, + new.memory_type, + new.scope, + new.subject, + CASE + WHEN json_valid(new.payload_json) THEN + COALESCE(json_extract(new.payload_json, '$.value'), '') || ' ' || + COALESCE(json_extract(new.payload_json, '$.note'), '') || ' ' || + COALESCE(json_extract(new.payload_json, '$.signal'), '') || ' ' || + COALESCE(json_extract(new.payload_json, '$.limit_value'), '') || ' ' || + COALESCE(json_extract(new.payload_json, '$.aliases'), '') || ' ' || + COALESCE(json_extract(new.payload_json, '$.text'), '') || ' ' || + COALESCE(json_extract(new.payload_json, '$.capture_type'), '') + ELSE '' + END, + new.source, + new.reason + ); +END; + +CREATE TRIGGER IF NOT EXISTS memories_au_fts AFTER UPDATE OF memory_type, scope, subject, payload_json, source, reason ON memories BEGIN + DELETE FROM memory_fts WHERE rowid = old.id; + INSERT INTO memory_fts(rowid, memory_type, scope, subject, payload_text, source, reason) + VALUES ( + new.id, + new.memory_type, + new.scope, + new.subject, + CASE + WHEN json_valid(new.payload_json) THEN + COALESCE(json_extract(new.payload_json, '$.value'), '') || ' ' || + COALESCE(json_extract(new.payload_json, '$.note'), '') || ' ' || + COALESCE(json_extract(new.payload_json, '$.signal'), '') || ' ' || + COALESCE(json_extract(new.payload_json, '$.limit_value'), '') || ' ' || + COALESCE(json_extract(new.payload_json, '$.aliases'), '') || ' ' || + COALESCE(json_extract(new.payload_json, '$.text'), '') || ' ' || + COALESCE(json_extract(new.payload_json, '$.capture_type'), '') + ELSE '' + END, + new.source, + new.reason + ); +END; + +CREATE TRIGGER IF NOT EXISTS memories_ad_fts AFTER DELETE ON memories BEGIN + DELETE FROM memory_fts WHERE rowid = old.id; +END; diff --git a/scripts/rebuild_memory_fts.py b/scripts/rebuild_memory_fts.py index 95052aa..619f9c6 100644 --- a/scripts/rebuild_memory_fts.py +++ b/scripts/rebuild_memory_fts.py @@ -26,7 +26,9 @@ def rebuild_memory_fts(db_path: Path) -> int: COALESCE(json_extract(payload_json, '$.note'), '') || ' ' || COALESCE(json_extract(payload_json, '$.signal'), '') || ' ' || COALESCE(json_extract(payload_json, '$.limit_value'), '') || ' ' || - COALESCE(json_extract(payload_json, '$.aliases'), '') + COALESCE(json_extract(payload_json, '$.aliases'), '') || ' ' || + COALESCE(json_extract(payload_json, '$.text'), '') || ' ' || + COALESCE(json_extract(payload_json, '$.capture_type'), '') ELSE '' END, source, diff --git a/tests/test_core_memory_tools.py b/tests/test_core_memory_tools.py index 1adcaa3..fd92c0f 100644 --- a/tests/test_core_memory_tools.py +++ b/tests/test_core_memory_tools.py @@ -34,6 +34,7 @@ def test_memory_tools_round_trip(tmp_path: Path) -> None: "memory_list", "memory_get", "memory_create", + "memory_capture", "memory_confirm", "memory_reject", "memory_expire", @@ -117,6 +118,121 @@ def test_memory_create_secret_block_surfaces_invalid_input_without_secret(tmp_pa assert secret not in str(blocked) +def test_memory_capture_happy_path_candidate(tmp_path: Path) -> None: + db_path = tmp_path / "m.db" + get_connection(db_path).close() + server = create_core_server(MinxTestConfig(db_path, tmp_path / "vault")) + capture_fn = get_tool(server, "memory_capture").fn + + out = capture_fn( + text="Pick up laundry after 5pm", + capture_type="observation", + scope="core", + subject=None, + source="user:capture", + confidence=0.5, + metadata=None, + ) + + assert out["success"] is True + memory = out["data"]["memory"] + assert memory["memory_type"] == "captured_thought" + assert memory["status"] == "candidate" + assert memory["confidence"] == 0.5 + assert out["data"]["response_template"] == "memory_capture.created_candidate" + assert out["data"]["response_slots"] == { + "memory_id": memory["id"], + "status": "candidate", + "memory_type": "captured_thought", + "scope": "core", + "subject": memory["subject"], + "capture_type": "observation", + } + assert memory["payload"] == { + "text": "Pick up laundry after 5pm", + "capture_type": "observation", + } + + +def test_memory_capture_with_metadata_and_explicit_subject(tmp_path: Path) -> None: + db_path = tmp_path / "m.db" + get_connection(db_path).close() + server = create_core_server(MinxTestConfig(db_path, tmp_path / "vault")) + capture_fn = get_tool(server, "memory_capture").fn + + out = capture_fn( + text="body text", + capture_type="todo", + scope="core", + subject="my_subject", + source="user:capture", + confidence=0.4, + metadata={"src": "chat"}, + ) + + assert out["success"] is True + memory = out["data"]["memory"] + assert memory["subject"] == "my_subject" + assert memory["payload"] == { + "text": "body text", + "capture_type": "todo", + "metadata": {"src": "chat"}, + } + + +def test_memory_capture_rejects_high_confidence(tmp_path: Path) -> None: + db_path = tmp_path / "m.db" + get_connection(db_path).close() + server = create_core_server(MinxTestConfig(db_path, tmp_path / "vault")) + capture_fn = get_tool(server, "memory_capture").fn + + out = capture_fn("x", "observation", "core", None, "user:capture", 0.8, None) + + assert out["success"] is False + assert out["error_code"] == "INVALID_INPUT" + + +def test_memory_capture_rejects_invalid_metadata(tmp_path: Path) -> None: + db_path = tmp_path / "m.db" + get_connection(db_path).close() + server = create_core_server(MinxTestConfig(db_path, tmp_path / "vault")) + capture_fn = get_tool(server, "memory_capture").fn + + out = capture_fn("x", "observation", "core", None, "user:capture", 0.5, {"bad": {1, 2}}) + + assert out["success"] is False + assert out["error_code"] == "INVALID_INPUT" + + +def test_memory_capture_secret_blocked_like_create(tmp_path: Path) -> None: + db_path = tmp_path / "m.db" + get_connection(db_path).close() + server = create_core_server(MinxTestConfig(db_path, tmp_path / "vault")) + capture_fn = get_tool(server, "memory_capture").fn + secret = _fake_private_key_block() + + out = capture_fn(secret, "observation", "core", None, "user:capture", 0.5, None) + + assert out["success"] is False + assert out["error_code"] == "INVALID_INPUT" + assert secret not in str(out) + + +def test_memory_capture_derived_subject_stable(tmp_path: Path) -> None: + db_path = tmp_path / "m.db" + get_connection(db_path).close() + server = create_core_server(MinxTestConfig(db_path, tmp_path / "vault")) + capture_fn = get_tool(server, "memory_capture").fn + + first = capture_fn(" hello \nworld", "Note", "core", None, "user:capture", 0.5, None) + second = capture_fn(" hello \nworld", "Note", "core", None, "user:capture", 0.5, None) + + assert first["success"] is True + assert second["success"] is False + assert second["error_code"] == "CONFLICT" + assert first["data"]["memory"]["subject"] == "note:hello" + + def test_memory_create_redacted_payload_returns_redacted_memory(tmp_path: Path) -> None: db_path = tmp_path / "m.db" get_connection(db_path).close() diff --git a/tests/test_goal_capture.py b/tests/test_goal_capture.py index a196150..ce0b3bb 100644 --- a/tests/test_goal_capture.py +++ b/tests/test_goal_capture.py @@ -119,6 +119,15 @@ def test_capture_goal_message_create_with_known_category(tmp_path): assert result.result_type == "create" assert result.action == "goal_create" + assert result.response_template == "goal_parse.create.ready" + assert result.response_slots == { + "action": "goal_create", + "goal_type": "spending_cap", + "subject": "Dining Out", + "subject_kind": "category", + "period": "monthly", + "target_value": 20000, + } assert result.payload is not None assert result.payload["category_names"] == ["Dining Out"] assert result.payload["target_value"] == 20000 @@ -163,6 +172,12 @@ def test_capture_goal_message_create_ambiguous_subject(tmp_path): assert result.result_type == "clarify" assert result.clarification_type == "ambiguous_subject" + assert result.clarification_template == "goal_parse.clarify.ambiguous_subject" + assert result.clarification_slots == { + "action": "goal_create", + "field": "subject", + "candidate_count": 2, + } assert result.options is not None assert len(result.options) == 2 @@ -198,6 +213,8 @@ def test_capture_goal_message_create_no_dollar_amount(tmp_path): ) assert result.result_type == "no_match" + assert result.response_template == "goal_parse.no_match.unsupported" + assert result.response_slots == {"status": "unsupported"} def test_capture_goal_message_no_goal_intent(tmp_path): diff --git a/tests/test_goal_parse.py b/tests/test_goal_parse.py index 0ff19d0..2e13da1 100644 --- a/tests/test_goal_parse.py +++ b/tests/test_goal_parse.py @@ -49,6 +49,15 @@ def test_goal_parse_tool_supports_structured_create_input(tmp_path) -> None: assert result["success"] is True assert result["data"]["result_type"] == "create" + assert result["data"]["response_template"] == "goal_parse.create.ready" + assert result["data"]["response_slots"] == { + "action": "goal_create", + "goal_type": "spending_cap", + "subject": "Dining Out", + "subject_kind": "category", + "period": "monthly", + "target_value": 25000, + } assert result["data"]["payload"]["category_names"] == ["Dining Out"] @@ -117,6 +126,12 @@ def test_goal_parse_tool_supports_structured_update_input(tmp_path) -> None: assert result["success"] is True assert result["data"]["result_type"] == "update" assert result["data"]["goal_id"] == 1 + assert result["data"]["response_template"] == "goal_parse.update.ready" + assert result["data"]["response_slots"] == { + "action": "goal_update", + "goal_id": 1, + "status": "paused", + } def test_goal_parse_tool_rejects_non_object_structured_input(tmp_path) -> None: @@ -233,6 +248,8 @@ def test_goal_parse_tool_returns_no_match_for_unsupported_structured_create_fami assert result["success"] is True assert result["data"]["result_type"] == "no_match" + assert result["data"]["response_template"] == "goal_parse.no_match.unsupported" + assert result["data"]["response_slots"] == {"status": "unsupported"} def test_goal_parse_tool_accepts_merchant_alias_that_normalizes_to_canonical(tmp_path) -> None: diff --git a/tests/test_goal_parse_llm_fallback.py b/tests/test_goal_parse_llm_fallback.py index e703cc4..e06654b 100644 --- a/tests/test_goal_parse_llm_fallback.py +++ b/tests/test_goal_parse_llm_fallback.py @@ -24,6 +24,23 @@ async def run_json_prompt(self, prompt): raise LLMError("Interpretation schema validation failed") +class _CreateLLM: + async def run_structured_prompt(self, prompt, result_model): + return { + "intent": "create", + "confidence": 0.9, + "subject_kind": "category", + "subject": "Dining Out", + "period": "monthly", + "target_value": 10000, + "update_kind": None, + "goal_id": None, + } + + async def run_json_prompt(self, prompt): + raise AssertionError("structured prompt should be used") + + class _StubFinanceRead: def get_spending_summary(self, start_date: str, end_date: str): return {} @@ -112,3 +129,25 @@ async def test_llm_none_falls_back_to_deterministic_regex() -> None: ) assert result.result_type == "create" + + +@pytest.mark.asyncio +async def test_render_fields_do_not_reuse_assistant_message_from_llm_path(monkeypatch) -> None: + chatty = "Woohoo, I made you a shiny goal!" + monkeypatch.setattr( + "minx_mcp.core.goal_capture_llm._build_create_assistant_message", + lambda subject: chatty, + ) + + result = await capture_goal_message( + message="spend less than $100 on Dining Out monthly", + review_date="2026-04-12", + finance_api=_StubFinanceRead(), + goals=[], + llm=_CreateLLM(), + ) + + assert result.assistant_message == chatty + assert result.response_template == "goal_parse.create.ready" + assert chatty not in str(result.response_template) + assert chatty not in str(result.response_slots) diff --git a/tests/test_memory_service.py b/tests/test_memory_service.py index e3e3dc7..e610dc7 100644 --- a/tests/test_memory_service.py +++ b/tests/test_memory_service.py @@ -10,7 +10,14 @@ import pytest from minx_mcp.contracts import ConflictError, InvalidInputError, NotFoundError -from minx_mcp.core.memory_models import MemoryProposal +from minx_mcp.core.memory_capture import ( + build_capture_response_slots, + derive_capture_subject, + normalize_capture_text_for_body, + normalize_capture_type, + validate_capture_metadata, +) +from minx_mcp.core.memory_models import MemoryProposal, MemoryRecord from minx_mcp.core.memory_service import MemoryService from minx_mcp.db import get_connection, migration_dir @@ -35,6 +42,99 @@ def _fake_private_key_block() -> str: ) +def test_normalize_capture_text_for_body_collapses_whitespace() -> None: + assert normalize_capture_text_for_body(" Buy milk\n tomorrow ") == "Buy milk tomorrow" + + +def test_normalize_capture_type_empty_becomes_observation() -> None: + assert normalize_capture_type("") == "observation" + assert normalize_capture_type(" ") == "observation" + + +def test_normalize_capture_type_sanitizes_and_lowercases() -> None: + assert normalize_capture_type(" Foo Bar ") == "foo_bar" + assert normalize_capture_type("Type-A/B") == "type-a_b" + + +def test_derive_capture_subject_stable_prefix_and_truncation() -> None: + capture_type = normalize_capture_type("observation") + raw_text = " Buy milk tomorrow \nsecond line ignored " + subject = derive_capture_subject( + capture_type_normalized=capture_type, + raw_text=raw_text, + explicit_subject=None, + ) + assert subject == "observation:Buy milk tomorrow" + + +def test_validate_capture_metadata_rejects_deep_nesting() -> None: + bad = {"a": {"b": {"c": {"d": {"e": "too deep"}}}}} + + with pytest.raises(InvalidInputError): + validate_capture_metadata(bad) + + +def test_validate_capture_metadata_rejects_long_string_leaf() -> None: + bad = {"k": "x" * 5000} + + with pytest.raises(InvalidInputError): + validate_capture_metadata(bad) + + +def test_validate_capture_metadata_rejects_non_json_leaf() -> None: + with pytest.raises(InvalidInputError): + validate_capture_metadata({"bad": object()}) + + +def test_build_capture_response_slots_returns_render_data_only() -> None: + record = MemoryRecord( + id=7, + memory_type="captured_thought", + scope="core", + subject="observation:Buy milk", + confidence=0.5, + status="candidate", + payload={"text": "Buy milk", "capture_type": "observation"}, + source="user:capture", + reason="", + created_at="2026-04-28T00:00:00Z", + updated_at="2026-04-28T00:00:00Z", + last_confirmed_at=None, + expires_at=None, + ) + assert build_capture_response_slots(record=record, capture_type="observation") == { + "memory_id": 7, + "status": "candidate", + "memory_type": "captured_thought", + "scope": "core", + "subject": "observation:Buy milk", + "capture_type": "observation", + } + + +def test_captured_thought_round_trip_list_and_search_candidate(tmp_path) -> None: + svc = _fresh_memory_service(tmp_path) + marker = "uniquewxyzzy913" + record = svc.create_memory( + memory_type="captured_thought", + scope="core", + subject="observation:test", + confidence=0.5, + payload={"text": f"note about {marker}", "capture_type": "observation"}, + source="user", + reason="", + ) + + listed = svc.list_memories(status="candidate", memory_type="captured_thought") + assert any(row.id == record.id for row in listed) + hits = svc.search_memories( + query=marker, + status="candidate", + memory_type="captured_thought", + ) + assert [result.memory.id for result in hits] == [record.id] + + def _proxy_conn_first_memory_id_select( inner: sqlite3.Connection, *, @@ -601,7 +701,8 @@ def test_migration_set_includes_015_memories_unique_live() -> None: assert "023_enrichment_queue.sql" in names assert "024_memory_embeddings.sql" in names assert "025_memory_fts_aliases.sql" in names - assert names[-1] == "025_memory_fts_aliases.sql" + assert "026_memory_capture_fts.sql" in names + assert names[-1] == "026_memory_capture_fts.sql" def test_unique_index_rejects_duplicate_live_triple(tmp_path) -> None: diff --git a/tests/test_rebuild_memory_fts.py b/tests/test_rebuild_memory_fts.py index efef1c2..545b5eb 100644 --- a/tests/test_rebuild_memory_fts.py +++ b/tests/test_rebuild_memory_fts.py @@ -61,6 +61,29 @@ def test_rebuild_memory_fts_indexes_entity_fact_aliases(tmp_path) -> None: assert [result.memory.id for result in svc.search_memories(query="corner")] == [record.id] +def test_rebuild_memory_fts_indexes_captured_thought_text(tmp_path) -> None: + db_path = tmp_path / "m.db" + svc = _service_for(db_path) + unique = "xenonbravo_capture_token_91357" + record = svc.create_memory( + memory_type="captured_thought", + scope="core", + subject="note:hello", + confidence=0.5, + payload={"text": f"Remember to find {unique} in FTS.", "capture_type": "observation"}, + source="user:test", + reason="", + ) + svc.conn.execute("DELETE FROM memory_fts WHERE rowid = ?", (record.id,)) + svc.conn.commit() + assert svc.search_memories(query=unique, status="candidate") == [] + + assert main([str(db_path)]) == 0 + + hits = svc.search_memories(query=unique, status="candidate") + assert [result.memory.id for result in hits] == [record.id] + + def test_rebuild_memory_fts_replaces_stale_rows(tmp_path) -> None: db_path = tmp_path / "m.db" svc = _service_for(db_path)