gitlore/docs/plan-expose-discussion-ids.feedback-5.md

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<i64>,
@@ ## 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<String>,
@@ ## 3d. Filters struct
+    pub contains: Option<String>,
@@ ## 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.