Your iteration 4 plan is already strong. The highest-impact revisions are around query shape, transaction boundaries, and contract stability for agents. 1. **Switch discussions query to a two-phase page-first architecture** Analysis: Current `ranked_notes` runs over every filtered discussion before `LIMIT`, which can explode on project-wide queries. A page-first plan keeps complexity proportional to `limit`, improves tail latency, and reduces memory churn. ```diff @@ ## 3c. SQL Query -Core query uses a CTE + ranked-notes rollup (window function) to avoid per-row correlated -subqueries. +Core query is split into two phases for scalability: +1) `paged_discussions` applies filters/sort/LIMIT and returns only page IDs. +2) Note rollups and optional `--include-notes` expansion run only for those page IDs. +This bounds note scanning to visible results and stabilizes latency on large projects. -WITH filtered_discussions AS ( +WITH filtered_discussions AS ( ... ), -ranked_notes AS ( +paged_discussions AS ( + SELECT id + FROM filtered_discussions + ORDER BY COALESCE({sort_column}, 0) {order}, id {order} + LIMIT ? +), +ranked_notes AS ( ... - WHERE n.discussion_id IN (SELECT id FROM filtered_discussions) + WHERE n.discussion_id IN (SELECT id FROM paged_discussions) ) ``` 2. **Move snapshot transaction ownership to handlers (not query helpers)** Analysis: This avoids nested transaction edge cases, keeps function signatures clean, and guarantees one snapshot across count + page + include-notes + serialization metadata. ```diff @@ ## Cross-cutting: snapshot consistency -Wrap `query_notes` and `query_discussions` in a deferred read transaction. +Open one deferred read transaction in each handler (`handle_notes`, `handle_discussions`) +and pass `&Transaction` into query helpers. Query helpers do not open/commit transactions. +This guarantees a single snapshot across all subqueries and avoids nested tx pitfalls. -pub fn query_discussions(conn: &Connection, ...) +pub fn query_discussions(tx: &rusqlite::Transaction<'_>, ...) ``` 3. **Add immutable input filter `--project-id` across notes/discussions/show** Analysis: You already expose `gitlab_project_id` because paths are mutable; input should support the same immutable selector. This removes failure modes after project renames/transfers. ```diff @@ ## 3a. CLI Args + /// Filter by immutable GitLab project ID + #[arg(long, help_heading = "Filters", conflicts_with = "project")] + pub project_id: Option, @@ ## Bridge Contract +Input symmetry rule: commands that accept `--project` should also accept `--project-id`. +If both are present, return usage error (exit code 2). ``` 4. **Enforce bridge fields for nested notes in `discussions --include-notes`** Analysis: Current guardrail is entity-level; nested notes can still lose required IDs under aggressive filtering. This is a contract hole for write-bridging. ```diff @@ ### Field Filtering Guardrail -In robot mode, `filter_fields` MUST force-include Bridge Contract fields... +In robot mode, `filter_fields` MUST force-include Bridge Contract fields at all returned levels: +- discussion row fields +- nested note fields when `discussions --include-notes` is used +const BRIDGE_FIELDS_DISCUSSION_NOTES: &[&str] = &[ + "project_path", "gitlab_project_id", "noteable_type", "parent_iid", + "gitlab_discussion_id", "gitlab_note_id", +]; ``` 5. **Make ambiguity preflight scope-aware and machine-actionable** Analysis: Current preflight checks only `gitlab_discussion_id`, which can produce false ambiguity when additional filters already narrow to one project. Also, agents need structured candidates, not only free-text. ```diff @@ ### Ambiguity Guardrail -SELECT DISTINCT p.path_with_namespace +SELECT DISTINCT p.path_with_namespace, p.gitlab_project_id FROM discussions d JOIN projects p ON p.id = d.project_id -WHERE d.gitlab_discussion_id = ? +WHERE d.gitlab_discussion_id = ? + /* plus active scope filters: noteable_type, for_issue/for_mr, since/path when present */ LIMIT 3 -Return LoreError::Ambiguous with message +Return LoreError::Ambiguous with structured details: +`{ code, message, candidates:[{project_path, gitlab_project_id}], suggestion }` ``` 6. **Add `--contains` filter to `discussions`** Analysis: This is a high-utility agent workflow gap. Agents frequently need “find thread by text then reply”; forcing a separate `notes` search round-trip is unnecessary. ```diff @@ ## 3a. CLI Args + /// Filter discussions whose notes contain text + #[arg(long, help_heading = "Filters")] + pub contains: Option, @@ ## 3d. Filters struct + pub contains: Option, @@ ## 3d. Where-clause construction +- `path` -> EXISTS (...) +- `path` -> EXISTS (...) +- `contains` -> EXISTS ( + SELECT 1 FROM notes n + WHERE n.discussion_id = d.id + AND n.body LIKE ? + ) ``` 7. **Promote two baseline indexes from “candidate” to “required”** Analysis: These are directly hit by new primary paths; waiting for post-merge profiling risks immediate perf cliffs in real usage. ```diff @@ ## 3h. Query-plan validation -Candidate indexes (add only if EXPLAIN QUERY PLAN shows they're needed): -- discussions(project_id, gitlab_discussion_id) -- notes(discussion_id, created_at DESC, id DESC) +Required baseline indexes for this feature: +- discussions(project_id, gitlab_discussion_id) +- notes(discussion_id, created_at DESC, id DESC) +Keep other indexes conditional on EXPLAIN QUERY PLAN. ``` 8. **Add schema versioning and remove contradictory rejected items** Analysis: `robot-docs` contract drift is a long-term agent risk; explicit schema versions let clients fail safely. Also, rejected items currently contradict active sections, which creates implementation ambiguity. ```diff @@ ## 4. Fix Robot-Docs Response Schemas "meta": {"elapsed_ms": "int", ...} +"meta": {"elapsed_ms":"int", ..., "schema_version":"string"} + +Schema version policy: +- bump minor on additive fields +- bump major on removals/renames +- expose per-command versions in `robot-docs` @@ ## Rejected Recommendations -- Add `gitlab_note_id` to show-command note detail structs ... rejected ... -- Add `gitlab_discussion_id` to show-command discussion detail structs ... rejected ... -- Add `gitlab_project_id` to show-command discussion detail structs ... rejected ... +Remove stale rejected entries that conflict with accepted workstreams in this plan iteration. ``` If you want, I can produce a fully rewritten iteration 5 plan document that applies all of the above edits cleanly end-to-end.