feat(cli): implement 'lore trace' command (bd-2n4, bd-9dd)
Gate 5 Code Trace - Tier 1 (API-only, no git blame). Answers 'Why was this code introduced?' by building file -> MR -> issue -> discussion chains. New files: - src/core/trace.rs: run_trace() query logic with rename-aware path resolution, entity_reference-based issue linking, and DiffNote discussion extraction - src/core/trace_tests.rs: 7 unit tests for query logic - src/cli/commands/trace.rs: CLI command with human output, robot JSON output, and :line suffix parsing (5 tests) Wiring: - TraceArgs + Commands::Trace in cli/mod.rs - handle_trace in main.rs - VALID_COMMANDS + robot-docs manifest entry - COMMAND_FLAGS autocorrect registry entry Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
File diff suppressed because one or more lines are too long
@@ -1 +1 @@
|
|||||||
bd-z94
|
bd-3jqx
|
||||||
|
|||||||
147
docs/plan-expose-discussion-ids.feedback-3.md
Normal file
147
docs/plan-expose-discussion-ids.feedback-3.md
Normal file
@@ -0,0 +1,147 @@
|
|||||||
|
1. **Make `gitlab_note_id` explicit in all note-level payloads without breaking existing consumers**
|
||||||
|
Rationale: Your Bridge Contract already requires `gitlab_note_id`, but current plan keeps `gitlab_id` only in `notes` list while adding `gitlab_note_id` only in `show`. That forces agents to special-case commands. Add `gitlab_note_id` as an alias field everywhere note-level data appears, while keeping `gitlab_id` for compatibility.
|
||||||
|
|
||||||
|
```diff
|
||||||
|
@@ Bridge Contract (Cross-Cutting)
|
||||||
|
-Every read payload that surfaces notes or discussions MUST include:
|
||||||
|
+Every read payload that surfaces notes or discussions MUST include:
|
||||||
|
- project_path
|
||||||
|
- noteable_type
|
||||||
|
- parent_iid
|
||||||
|
- gitlab_discussion_id
|
||||||
|
- gitlab_note_id (when note-level data is returned — i.e., in notes list and show detail)
|
||||||
|
+ - Back-compat rule: note payloads may continue exposing `gitlab_id`, but MUST also expose `gitlab_note_id` with the same value.
|
||||||
|
|
||||||
|
@@ 1. Add `gitlab_discussion_id` to Notes Output
|
||||||
|
-#### 1c. Add field to `NoteListRowJson`
|
||||||
|
+#### 1c. Add fields to `NoteListRowJson`
|
||||||
|
+Add `gitlab_note_id` alias in addition to existing `gitlab_id` (no rename, no breakage).
|
||||||
|
|
||||||
|
@@ 1f. Update `--fields minimal` preset
|
||||||
|
-"notes" => ["id", "author_username", "body", "created_at_iso", "gitlab_discussion_id"]
|
||||||
|
+"notes" => ["id", "gitlab_note_id", "author_username", "body", "created_at_iso", "gitlab_discussion_id"]
|
||||||
|
```
|
||||||
|
|
||||||
|
2. **Avoid duplicate flag semantics for discussion filtering**
|
||||||
|
Rationale: `notes` already has `--discussion-id` and it already maps to `d.gitlab_discussion_id`. Adding a second independent flag/field (`--gitlab-discussion-id`) increases complexity and precedence bugs. Keep one backing filter field and make the new flag an alias.
|
||||||
|
|
||||||
|
```diff
|
||||||
|
@@ 1g. Add `--gitlab-discussion-id` filter to notes
|
||||||
|
-Allow filtering notes directly by GitLab discussion thread ID...
|
||||||
|
+Normalize discussion ID flags:
|
||||||
|
+- Keep one backing filter field (`discussion_id`)
|
||||||
|
+- Support both `--discussion-id` (existing) and `--gitlab-discussion-id` (alias)
|
||||||
|
+- If both are provided, clap should reject as duplicate/alias conflict
|
||||||
|
```
|
||||||
|
|
||||||
|
3. **Add ambiguity guardrails for cross-project discussion IDs**
|
||||||
|
Rationale: `gitlab_discussion_id` is unique per project, not globally. Filtering by discussion ID without project can return multiple rows across repos, which breaks deterministic write bridging. Fail fast with an `Ambiguous` error and actionable fix (`--project`).
|
||||||
|
|
||||||
|
```diff
|
||||||
|
@@ Bridge Contract (Cross-Cutting)
|
||||||
|
+### Ambiguity Guardrail
|
||||||
|
+When filtering by `gitlab_discussion_id` without `--project`, if multiple projects match:
|
||||||
|
+- return `Ambiguous` error
|
||||||
|
+- include matching project paths in message
|
||||||
|
+- suggest retry with `--project <path>`
|
||||||
|
```
|
||||||
|
|
||||||
|
4. **Replace `--include-notes` N+1 retrieval with one batched top-N query**
|
||||||
|
Rationale: The current plan’s per-discussion follow-up query scales poorly and creates latency spikes. Use a single window-function query over selected discussion IDs and group rows in Rust. This is both faster and more predictable.
|
||||||
|
|
||||||
|
```diff
|
||||||
|
@@ 3c-ii. Note expansion query (--include-notes)
|
||||||
|
-When `include_notes > 0`, after the main discussion query, run a follow-up query per discussion...
|
||||||
|
+When `include_notes > 0`, run one batched query:
|
||||||
|
+WITH ranked_notes AS (
|
||||||
|
+ SELECT
|
||||||
|
+ n.*,
|
||||||
|
+ d.gitlab_discussion_id,
|
||||||
|
+ ROW_NUMBER() OVER (
|
||||||
|
+ PARTITION BY n.discussion_id
|
||||||
|
+ ORDER BY n.created_at DESC, n.id DESC
|
||||||
|
+ ) AS rn
|
||||||
|
+ FROM notes n
|
||||||
|
+ JOIN discussions d ON d.id = n.discussion_id
|
||||||
|
+ WHERE n.discussion_id IN ( ...selected discussion ids... )
|
||||||
|
+)
|
||||||
|
+SELECT ... FROM ranked_notes WHERE rn <= ?
|
||||||
|
+ORDER BY discussion_id, rn;
|
||||||
|
+
|
||||||
|
+Group by `discussion_id` in Rust and attach notes arrays without per-thread round-trips.
|
||||||
|
```
|
||||||
|
|
||||||
|
5. **Add hard output guardrails and explicit truncation metadata**
|
||||||
|
Rationale: `--limit` and `--include-notes` are unbounded today. For robot workflows this can accidentally generate huge payloads. Cap values and surface effective limits plus truncation state in `meta`.
|
||||||
|
|
||||||
|
```diff
|
||||||
|
@@ 3a. CLI Args
|
||||||
|
- pub limit: usize,
|
||||||
|
+ pub limit: usize, // clamp to max (e.g., 500)
|
||||||
|
|
||||||
|
- pub include_notes: usize,
|
||||||
|
+ pub include_notes: usize, // clamp to max (e.g., 20)
|
||||||
|
|
||||||
|
@@ Response Schema
|
||||||
|
- "meta": { "elapsed_ms": 12 }
|
||||||
|
+ "meta": {
|
||||||
|
+ "elapsed_ms": 12,
|
||||||
|
+ "effective_limit": 50,
|
||||||
|
+ "effective_include_notes": 2,
|
||||||
|
+ "has_more": true
|
||||||
|
+ }
|
||||||
|
```
|
||||||
|
|
||||||
|
6. **Strengthen deterministic ordering and null handling**
|
||||||
|
Rationale: `first_note_at`, `last_note_at`, and note `position` can be null/incomplete during partial sync states. Add null-safe ordering to avoid unstable output and flaky automation.
|
||||||
|
|
||||||
|
```diff
|
||||||
|
@@ 2c. Update queries to SELECT new fields
|
||||||
|
-... ORDER BY first_note_at
|
||||||
|
+... ORDER BY COALESCE(first_note_at, last_note_at, 0), id
|
||||||
|
|
||||||
|
@@ show note query
|
||||||
|
-ORDER BY position
|
||||||
|
+ORDER BY COALESCE(position, 9223372036854775807), created_at, id
|
||||||
|
|
||||||
|
@@ 3c. SQL Query
|
||||||
|
-ORDER BY {sort_column} {order}
|
||||||
|
+ORDER BY COALESCE({sort_column}, 0) {order}, fd.id {order}
|
||||||
|
```
|
||||||
|
|
||||||
|
7. **Make write-bridging more useful with optional command hints**
|
||||||
|
Rationale: Exposing IDs is necessary but not sufficient; agents still need to assemble endpoints repeatedly. Add optional `--with-write-hints` that injects compact endpoint templates (`reply`, `resolve`) derived from row context. This improves usability without bloating default output.
|
||||||
|
|
||||||
|
```diff
|
||||||
|
@@ 3a. CLI Args
|
||||||
|
+ /// Include machine-actionable glab write hints per row
|
||||||
|
+ #[arg(long, help_heading = "Output")]
|
||||||
|
+ pub with_write_hints: bool,
|
||||||
|
|
||||||
|
@@ Response Schema (notes/discussions/show)
|
||||||
|
+ "write_hints?": {
|
||||||
|
+ "reply_endpoint": "string",
|
||||||
|
+ "resolve_endpoint?": "string"
|
||||||
|
+ }
|
||||||
|
```
|
||||||
|
|
||||||
|
8. **Upgrade robot-docs/contract validation from string-contains to parity checks**
|
||||||
|
Rationale: `contains("gitlab_discussion_id")` catches very little and allows schema drift. Build field-set parity tests that compare actual serialized JSON keys to robot-docs declared fields for `notes`, `discussions`, and `show` discussion nodes.
|
||||||
|
|
||||||
|
```diff
|
||||||
|
@@ 4f. Add robot-docs contract tests
|
||||||
|
-assert!(notes_schema.contains("gitlab_discussion_id"));
|
||||||
|
+let declared = parse_schema_field_list(notes_schema);
|
||||||
|
+let sample = sample_notes_row_json_keys();
|
||||||
|
+assert_required_subset(&declared, &["project_path","noteable_type","parent_iid","gitlab_discussion_id","gitlab_note_id"]);
|
||||||
|
+assert_schema_matches_payload(&declared, &sample);
|
||||||
|
|
||||||
|
@@ 4g. Add CLI-level contract integration tests
|
||||||
|
+Add parity tests for:
|
||||||
|
+- notes list JSON
|
||||||
|
+- discussions list JSON
|
||||||
|
+- issues show discussions[*]
|
||||||
|
+- mrs show discussions[*]
|
||||||
|
```
|
||||||
|
|
||||||
|
If you want, I can produce a full revised v3 plan text with these edits merged end-to-end so it’s ready to execute directly.
|
||||||
@@ -2,7 +2,7 @@
|
|||||||
plan: true
|
plan: true
|
||||||
title: ""
|
title: ""
|
||||||
status: iterating
|
status: iterating
|
||||||
iteration: 2
|
iteration: 3
|
||||||
target_iterations: 8
|
target_iterations: 8
|
||||||
beads_revision: 0
|
beads_revision: 0
|
||||||
related_plans: []
|
related_plans: []
|
||||||
@@ -34,6 +34,11 @@ Every read payload that surfaces notes or discussions **MUST** include:
|
|||||||
- `gitlab_discussion_id`
|
- `gitlab_discussion_id`
|
||||||
- `gitlab_note_id` (when note-level data is returned — i.e., in notes list and show detail)
|
- `gitlab_note_id` (when note-level data is returned — i.e., in notes list and show detail)
|
||||||
|
|
||||||
|
**Back-compat rule**: Note payloads in the `notes` list command continue exposing `gitlab_id`
|
||||||
|
for existing consumers, but **MUST also** expose `gitlab_note_id` with the same value. This
|
||||||
|
ensures agents can use a single field name (`gitlab_note_id`) across all commands — `notes`,
|
||||||
|
`show`, and `discussions --include-notes` — without special-casing by command.
|
||||||
|
|
||||||
This contract exists so agents can deterministically construct `glab api` write calls without
|
This contract exists so agents can deterministically construct `glab api` write calls without
|
||||||
cross-referencing multiple commands. Each workstream below must satisfy these fields in its
|
cross-referencing multiple commands. Each workstream below must satisfy these fields in its
|
||||||
output.
|
output.
|
||||||
@@ -64,6 +69,37 @@ In `filter_fields`, when entity is `"notes"` or `"discussions"`, merge the bridg
|
|||||||
requested fields before filtering the JSON value. This is a ~5-line change to the existing
|
requested fields before filtering the JSON value. This is a ~5-line change to the existing
|
||||||
function.
|
function.
|
||||||
|
|
||||||
|
### Ambiguity Guardrail
|
||||||
|
|
||||||
|
When filtering by `gitlab_discussion_id` (on either `notes` or `discussions` commands) without
|
||||||
|
`--project`, if the query matches discussions in multiple projects:
|
||||||
|
- Return an `Ambiguous` error (exit code 18, matching existing convention)
|
||||||
|
- Include matching project paths in the error message
|
||||||
|
- Suggest retry with `--project <path>`
|
||||||
|
|
||||||
|
**Implementation**: After the main query, if `gitlab_discussion_id` is set and no `--project`
|
||||||
|
was provided, check if the result set spans multiple `project_path` values. If so, return
|
||||||
|
`LoreError::Ambiguous` with the distinct project paths. This is a post-query check (not a
|
||||||
|
pre-query reject) so it only fires when real ambiguity exists.
|
||||||
|
|
||||||
|
```rust
|
||||||
|
// In query_notes / query_discussions, after collecting results:
|
||||||
|
if filters.gitlab_discussion_id.is_some() && filters.project.is_none() {
|
||||||
|
let distinct_projects: HashSet<&str> = results.iter()
|
||||||
|
.map(|r| r.project_path.as_str())
|
||||||
|
.collect();
|
||||||
|
if distinct_projects.len() > 1 {
|
||||||
|
return Err(LoreError::Ambiguous {
|
||||||
|
message: format!(
|
||||||
|
"Discussion ID matches {} projects: {}. Use --project to disambiguate.",
|
||||||
|
distinct_projects.len(),
|
||||||
|
distinct_projects.into_iter().collect::<Vec<_>>().join(", ")
|
||||||
|
),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 1. Add `gitlab_discussion_id` to Notes Output
|
## 1. Add `gitlab_discussion_id` to Notes Output
|
||||||
@@ -175,13 +211,17 @@ etc.) which rusqlite's `row.get("name")` can resolve. This eliminates the fragil
|
|||||||
column-index counting that has caused bugs in the past. If the conversion touches too many
|
column-index counting that has caused bugs in the past. If the conversion touches too many
|
||||||
lines, limit named lookup to just the new field and add a follow-up task.
|
lines, limit named lookup to just the new field and add a follow-up task.
|
||||||
|
|
||||||
#### 1c. Add field to `NoteListRowJson`
|
#### 1c. Add fields to `NoteListRowJson`
|
||||||
|
|
||||||
**File**: `src/cli/commands/list.rs` line ~1093
|
**File**: `src/cli/commands/list.rs` line ~1093
|
||||||
|
|
||||||
|
Add both `gitlab_discussion_id` and `gitlab_note_id` (alias for `gitlab_id`):
|
||||||
|
|
||||||
```rust
|
```rust
|
||||||
pub struct NoteListRowJson {
|
pub struct NoteListRowJson {
|
||||||
// ... existing fields ...
|
// ... existing fields ...
|
||||||
|
pub gitlab_id: i64, // KEEP — existing consumers
|
||||||
|
pub gitlab_note_id: i64, // ADD — Bridge Contract alias
|
||||||
pub project_path: String,
|
pub project_path: String,
|
||||||
pub gitlab_discussion_id: String, // ADD
|
pub gitlab_discussion_id: String, // ADD
|
||||||
}
|
}
|
||||||
@@ -194,6 +234,8 @@ impl From<&NoteListRow> for NoteListRowJson {
|
|||||||
fn from(row: &NoteListRow) -> Self {
|
fn from(row: &NoteListRow) -> Self {
|
||||||
Self {
|
Self {
|
||||||
// ... existing fields ...
|
// ... existing fields ...
|
||||||
|
gitlab_id: row.gitlab_id,
|
||||||
|
gitlab_note_id: row.gitlab_id, // ADD — same value as gitlab_id
|
||||||
project_path: row.project_path.clone(),
|
project_path: row.project_path.clone(),
|
||||||
gitlab_discussion_id: row.gitlab_discussion_id.clone(), // ADD
|
gitlab_discussion_id: row.gitlab_discussion_id.clone(), // ADD
|
||||||
}
|
}
|
||||||
@@ -205,7 +247,7 @@ impl From<&NoteListRow> for NoteListRowJson {
|
|||||||
|
|
||||||
**File**: `src/cli/commands/list.rs` line ~1004
|
**File**: `src/cli/commands/list.rs` line ~1004
|
||||||
|
|
||||||
Add `gitlab_discussion_id` to the CSV header and row output.
|
Add `gitlab_discussion_id` and `gitlab_note_id` to the CSV header and row output.
|
||||||
|
|
||||||
#### 1e. Add to table display
|
#### 1e. Add to table display
|
||||||
|
|
||||||
@@ -218,13 +260,13 @@ Add a column showing a truncated discussion ID (first 8 chars) in the table view
|
|||||||
**File**: `src/cli/robot.rs` line ~67
|
**File**: `src/cli/robot.rs` line ~67
|
||||||
|
|
||||||
```rust
|
```rust
|
||||||
"notes" => ["id", "author_username", "body", "created_at_iso", "gitlab_discussion_id"]
|
"notes" => ["id", "gitlab_note_id", "author_username", "body", "created_at_iso", "gitlab_discussion_id"]
|
||||||
.iter()
|
.iter()
|
||||||
.map(|s| (*s).to_string())
|
.map(|s| (*s).to_string())
|
||||||
.collect(),
|
.collect(),
|
||||||
```
|
```
|
||||||
|
|
||||||
The discussion ID is critical enough for agent workflows that it belongs in `minimal`.
|
The discussion ID and note ID are critical for agent bridge workflows and belong in `minimal`.
|
||||||
|
|
||||||
#### 1g. Add `--gitlab-discussion-id` filter to notes
|
#### 1g. Add `--gitlab-discussion-id` filter to notes
|
||||||
|
|
||||||
@@ -233,6 +275,10 @@ the internal integer). This enables one-hop note retrieval from external referen
|
|||||||
that received a `gitlab_discussion_id` from another command or webhook can jump straight to
|
that received a `gitlab_discussion_id` from another command or webhook can jump straight to
|
||||||
the relevant notes without knowing the internal discussion ID.
|
the relevant notes without knowing the internal discussion ID.
|
||||||
|
|
||||||
|
**Note**: This is distinct from the existing `--discussion-id` filter which takes the internal
|
||||||
|
integer ID. The two filters serve different use cases: internal cross-referencing vs. external
|
||||||
|
API bridging.
|
||||||
|
|
||||||
**File**: `src/cli/mod.rs` (NotesArgs)
|
**File**: `src/cli/mod.rs` (NotesArgs)
|
||||||
|
|
||||||
```rust
|
```rust
|
||||||
@@ -286,6 +332,7 @@ fn note_list_row_json_includes_gitlab_discussion_id() {
|
|||||||
|
|
||||||
let json_row = NoteListRowJson::from(&row);
|
let json_row = NoteListRowJson::from(&row);
|
||||||
assert_eq!(json_row.gitlab_discussion_id, "6a9c1750b37d");
|
assert_eq!(json_row.gitlab_discussion_id, "6a9c1750b37d");
|
||||||
|
assert_eq!(json_row.gitlab_note_id, 100); // alias matches gitlab_id
|
||||||
|
|
||||||
let serialized = serde_json::to_value(&json_row).unwrap();
|
let serialized = serde_json::to_value(&json_row).unwrap();
|
||||||
assert!(serialized.get("gitlab_discussion_id").is_some());
|
assert!(serialized.get("gitlab_discussion_id").is_some());
|
||||||
@@ -293,6 +340,9 @@ fn note_list_row_json_includes_gitlab_discussion_id() {
|
|||||||
serialized["gitlab_discussion_id"].as_str().unwrap(),
|
serialized["gitlab_discussion_id"].as_str().unwrap(),
|
||||||
"6a9c1750b37d"
|
"6a9c1750b37d"
|
||||||
);
|
);
|
||||||
|
// Both gitlab_id and gitlab_note_id present with same value
|
||||||
|
assert_eq!(serialized["gitlab_id"], 100);
|
||||||
|
assert_eq!(serialized["gitlab_note_id"], 100);
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
@@ -436,6 +486,19 @@ fn notes_filter_by_gitlab_discussion_id() {
|
|||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
|
#### Test 6: Ambiguity guardrail fires for cross-project discussion ID matches
|
||||||
|
|
||||||
|
```rust
|
||||||
|
#[test]
|
||||||
|
fn notes_ambiguous_gitlab_discussion_id_across_projects() {
|
||||||
|
let conn = create_test_db();
|
||||||
|
// Insert 2 projects, each with a discussion sharing the same gitlab_discussion_id
|
||||||
|
// (this can happen since IDs are per-project)
|
||||||
|
// Filter by gitlab_discussion_id without --project
|
||||||
|
// Assert LoreError::Ambiguous is returned with both project paths
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 2. Add `gitlab_discussion_id` to Show Command Discussion Groups
|
## 2. Add `gitlab_discussion_id` to Show Command Discussion Groups
|
||||||
@@ -534,18 +597,24 @@ pub struct MrDiscussionDetailJson {
|
|||||||
|
|
||||||
**Issue discussions** (`show.rs:325`):
|
**Issue discussions** (`show.rs:325`):
|
||||||
```sql
|
```sql
|
||||||
SELECT id, gitlab_discussion_id, individual_note, resolvable, resolved, last_note_at
|
SELECT id, gitlab_discussion_id, individual_note, resolvable, resolved,
|
||||||
|
COALESCE(last_note_at, first_note_at, 0) AS last_note_at
|
||||||
FROM discussions
|
FROM discussions
|
||||||
WHERE issue_id = ? ORDER BY first_note_at
|
WHERE issue_id = ? ORDER BY COALESCE(first_note_at, last_note_at, 0), id
|
||||||
```
|
```
|
||||||
|
|
||||||
**MR discussions** (`show.rs:537`):
|
**MR discussions** (`show.rs:537`):
|
||||||
```sql
|
```sql
|
||||||
SELECT id, gitlab_discussion_id, individual_note, resolvable, resolved, last_note_at
|
SELECT id, gitlab_discussion_id, individual_note, resolvable, resolved,
|
||||||
|
COALESCE(last_note_at, first_note_at, 0) AS last_note_at
|
||||||
FROM discussions
|
FROM discussions
|
||||||
WHERE merge_request_id = ? ORDER BY first_note_at
|
WHERE merge_request_id = ? ORDER BY COALESCE(first_note_at, last_note_at, 0), id
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**Note on ordering**: The `COALESCE` with tiebreaker `id` ensures deterministic ordering even
|
||||||
|
when timestamps are NULL (possible during partial sync states). This prevents unstable output
|
||||||
|
that could confuse automated workflows.
|
||||||
|
|
||||||
#### 2d. Update query_map closures
|
#### 2d. Update query_map closures
|
||||||
|
|
||||||
The `disc_rows` tuple changes from `(i64, bool)` to a richer shape. Use named columns here
|
The `disc_rows` tuple changes from `(i64, bool)` to a richer shape. Use named columns here
|
||||||
@@ -753,7 +822,12 @@ lore -J discussions --for-mr 99 --resolution unresolved --include-notes 2
|
|||||||
"total_count": 15,
|
"total_count": 15,
|
||||||
"showing": 15
|
"showing": 15
|
||||||
},
|
},
|
||||||
"meta": { "elapsed_ms": 12 }
|
"meta": {
|
||||||
|
"elapsed_ms": 12,
|
||||||
|
"effective_limit": 50,
|
||||||
|
"effective_include_notes": 0,
|
||||||
|
"has_more": false
|
||||||
|
}
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
@@ -761,6 +835,10 @@ The `notes` array is empty by default (zero overhead). When `--include-notes N`
|
|||||||
each discussion includes up to N of its most recent notes inline. This covers the common
|
each discussion includes up to N of its most recent notes inline. This covers the common
|
||||||
agent pattern of "show me unresolved threads with context" in a single round-trip.
|
agent pattern of "show me unresolved threads with context" in a single round-trip.
|
||||||
|
|
||||||
|
The `meta` block includes `effective_limit` and `effective_include_notes` (the clamped values
|
||||||
|
actually used) plus `has_more` (true when total_count > showing). This lets agents detect
|
||||||
|
truncation and decide whether to paginate or narrow their query.
|
||||||
|
|
||||||
### File Architecture
|
### File Architecture
|
||||||
|
|
||||||
**No new files.** Follow the existing pattern:
|
**No new files.** Follow the existing pattern:
|
||||||
@@ -789,7 +867,7 @@ Args struct:
|
|||||||
```rust
|
```rust
|
||||||
#[derive(Parser)]
|
#[derive(Parser)]
|
||||||
pub struct DiscussionsArgs {
|
pub struct DiscussionsArgs {
|
||||||
/// Maximum results
|
/// Maximum results (clamped to 500)
|
||||||
#[arg(short = 'n', long = "limit", default_value = "50", help_heading = "Output")]
|
#[arg(short = 'n', long = "limit", default_value = "50", help_heading = "Output")]
|
||||||
pub limit: usize,
|
pub limit: usize,
|
||||||
|
|
||||||
@@ -833,7 +911,7 @@ pub struct DiscussionsArgs {
|
|||||||
#[arg(long, value_parser = ["Issue", "MergeRequest"], help_heading = "Filters")]
|
#[arg(long, value_parser = ["Issue", "MergeRequest"], help_heading = "Filters")]
|
||||||
pub noteable_type: Option<String>,
|
pub noteable_type: Option<String>,
|
||||||
|
|
||||||
/// Include up to N latest notes per discussion (0 = none, default)
|
/// Include up to N latest notes per discussion (0 = none, default; clamped to 20)
|
||||||
#[arg(long, default_value = "0", help_heading = "Output")]
|
#[arg(long, default_value = "0", help_heading = "Output")]
|
||||||
pub include_notes: usize,
|
pub include_notes: usize,
|
||||||
|
|
||||||
@@ -847,6 +925,11 @@ pub struct DiscussionsArgs {
|
|||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**Output guardrails**: The handler clamps `limit` to `min(limit, 500)` and `include_notes`
|
||||||
|
to `min(include_notes, 20)` before passing to the query layer. This prevents accidentally
|
||||||
|
huge payloads in robot mode. The clamped values are reported in `meta.effective_limit` and
|
||||||
|
`meta.effective_include_notes`.
|
||||||
|
|
||||||
#### 3b. Domain Structs
|
#### 3b. Domain Structs
|
||||||
|
|
||||||
**File**: `src/cli/commands/list.rs`
|
**File**: `src/cli/commands/list.rs`
|
||||||
@@ -981,8 +1064,8 @@ SELECT
|
|||||||
COALESCE(nr.note_count, 0) AS note_count,
|
COALESCE(nr.note_count, 0) AS note_count,
|
||||||
nr.first_author,
|
nr.first_author,
|
||||||
nr.first_note_body,
|
nr.first_note_body,
|
||||||
fd.first_note_at,
|
COALESCE(fd.first_note_at, fd.last_note_at, 0) AS first_note_at,
|
||||||
fd.last_note_at,
|
COALESCE(fd.last_note_at, fd.first_note_at, 0) AS last_note_at,
|
||||||
fd.resolvable,
|
fd.resolvable,
|
||||||
fd.resolved,
|
fd.resolved,
|
||||||
nr.position_new_path,
|
nr.position_new_path,
|
||||||
@@ -992,7 +1075,7 @@ JOIN projects p ON fd.project_id = p.id
|
|||||||
LEFT JOIN issues i ON fd.issue_id = i.id
|
LEFT JOIN issues i ON fd.issue_id = i.id
|
||||||
LEFT JOIN merge_requests m ON fd.merge_request_id = m.id
|
LEFT JOIN merge_requests m ON fd.merge_request_id = m.id
|
||||||
LEFT JOIN note_rollup nr ON nr.discussion_id = fd.id
|
LEFT JOIN note_rollup nr ON nr.discussion_id = fd.id
|
||||||
ORDER BY {sort_column} {order}
|
ORDER BY COALESCE({sort_column}, 0) {order}, fd.id {order}
|
||||||
LIMIT ?
|
LIMIT ?
|
||||||
```
|
```
|
||||||
|
|
||||||
@@ -1004,39 +1087,54 @@ of discussions, the window function approach avoids repeated index probes and pr
|
|||||||
more predictable query plan. The `MAX(CASE WHEN rn = 1 ...)` pattern extracts first-note
|
more predictable query plan. The `MAX(CASE WHEN rn = 1 ...)` pattern extracts first-note
|
||||||
attributes from the grouped output without additional lookups.
|
attributes from the grouped output without additional lookups.
|
||||||
|
|
||||||
|
**Note on ordering**: The `COALESCE({sort_column}, 0)` with tiebreaker `fd.id` ensures
|
||||||
|
deterministic ordering even when timestamps are NULL (partial sync states). The `id`
|
||||||
|
tiebreaker is cheap (primary key) and prevents unstable sort output.
|
||||||
|
|
||||||
**Note on SQLite FILTER syntax**: SQLite does not support `COUNT(*) FILTER (WHERE ...)`.
|
**Note on SQLite FILTER syntax**: SQLite does not support `COUNT(*) FILTER (WHERE ...)`.
|
||||||
Use `SUM(CASE WHEN ... THEN 1 ELSE 0 END)` instead (as shown above).
|
Use `SUM(CASE WHEN ... THEN 1 ELSE 0 END)` instead (as shown above).
|
||||||
|
|
||||||
#### 3c-ii. Note expansion query (--include-notes)
|
#### 3c-ii. Note expansion query (--include-notes)
|
||||||
|
|
||||||
When `include_notes > 0`, after the main discussion query, run a follow-up query per
|
When `include_notes > 0`, after the main discussion query, run a **single batched query**
|
||||||
discussion to fetch its N most recent notes:
|
using a window function to fetch the N most recent notes per discussion:
|
||||||
|
|
||||||
```sql
|
```sql
|
||||||
SELECT n.id, n.gitlab_id, n.author_username, n.body, n.note_type,
|
WITH ranked_expansion AS (
|
||||||
n.is_system, n.created_at, n.updated_at,
|
SELECT
|
||||||
n.position_new_path, n.position_new_line,
|
n.id, n.gitlab_id, n.author_username, n.body, n.note_type,
|
||||||
n.position_old_path, n.position_old_line,
|
n.is_system, n.created_at, n.updated_at,
|
||||||
n.resolvable, n.resolved, n.resolved_by,
|
n.position_new_path, n.position_new_line,
|
||||||
d.noteable_type,
|
n.position_old_path, n.position_old_line,
|
||||||
COALESCE(i.iid, m.iid) AS parent_iid,
|
n.resolvable, n.resolved, n.resolved_by,
|
||||||
COALESCE(i.title, m.title) AS parent_title,
|
d.noteable_type,
|
||||||
p.path_with_namespace AS project_path,
|
COALESCE(i.iid, m.iid) AS parent_iid,
|
||||||
d.gitlab_discussion_id
|
COALESCE(i.title, m.title) AS parent_title,
|
||||||
FROM notes n
|
p.path_with_namespace AS project_path,
|
||||||
JOIN discussions d ON n.discussion_id = d.id
|
d.gitlab_discussion_id,
|
||||||
JOIN projects p ON n.project_id = p.id
|
n.discussion_id,
|
||||||
LEFT JOIN issues i ON d.issue_id = i.id
|
ROW_NUMBER() OVER (
|
||||||
LEFT JOIN merge_requests m ON d.merge_request_id = m.id
|
PARTITION BY n.discussion_id
|
||||||
WHERE d.id = ?
|
ORDER BY n.created_at DESC, n.id DESC
|
||||||
ORDER BY n.created_at DESC
|
) AS rn
|
||||||
LIMIT ?
|
FROM notes n
|
||||||
|
JOIN discussions d ON n.discussion_id = d.id
|
||||||
|
JOIN projects p ON n.project_id = p.id
|
||||||
|
LEFT JOIN issues i ON d.issue_id = i.id
|
||||||
|
LEFT JOIN merge_requests m ON d.merge_request_id = m.id
|
||||||
|
WHERE n.discussion_id IN ({placeholders})
|
||||||
|
)
|
||||||
|
SELECT * FROM ranked_expansion WHERE rn <= ?
|
||||||
|
ORDER BY discussion_id, rn
|
||||||
```
|
```
|
||||||
|
|
||||||
**Optimization**: If discussion count is small (<= 50), batch all discussion IDs into a
|
Group by `discussion_id` in Rust and attach notes arrays to the corresponding
|
||||||
single `WHERE d.id IN (?, ?, ...)` query with a secondary partition to split by discussion.
|
`DiscussionListRowJson`. This avoids per-discussion round-trips entirely — one query
|
||||||
For larger result sets, fall back to per-discussion queries to avoid huge IN clauses. This
|
regardless of how many discussions are in the result set.
|
||||||
matches the existing note-loading pattern in `show.rs`.
|
|
||||||
|
The `{placeholders}` are the `id` values from the main discussion query result. Since
|
||||||
|
the discussion count is already clamped by `--limit` (max 500), the IN clause size is
|
||||||
|
bounded and safe.
|
||||||
|
|
||||||
The returned `NoteListRow` rows reuse the same struct and `NoteListRowJson` conversion from
|
The returned `NoteListRow` rows reuse the same struct and `NoteListRowJson` conversion from
|
||||||
workstream 1, ensuring identical note shape across all commands.
|
workstream 1, ensuring identical note shape across all commands.
|
||||||
@@ -1093,8 +1191,10 @@ fn handle_discussions(
|
|||||||
let conn = create_connection(&db_path)?;
|
let conn = create_connection(&db_path)?;
|
||||||
|
|
||||||
let order = if args.asc { "asc" } else { "desc" };
|
let order = if args.asc { "asc" } else { "desc" };
|
||||||
|
let effective_limit = args.limit.min(500);
|
||||||
|
let effective_include_notes = args.include_notes.min(20);
|
||||||
let filters = DiscussionListFilters {
|
let filters = DiscussionListFilters {
|
||||||
limit: args.limit,
|
limit: effective_limit,
|
||||||
project: args.project,
|
project: args.project,
|
||||||
for_issue_iid: args.for_issue,
|
for_issue_iid: args.for_issue,
|
||||||
for_mr_iid: args.for_mr,
|
for_mr_iid: args.for_mr,
|
||||||
@@ -1105,7 +1205,7 @@ fn handle_discussions(
|
|||||||
noteable_type: args.noteable_type,
|
noteable_type: args.noteable_type,
|
||||||
sort: args.sort,
|
sort: args.sort,
|
||||||
order: order.to_string(),
|
order: order.to_string(),
|
||||||
include_notes: args.include_notes,
|
include_notes: effective_include_notes,
|
||||||
};
|
};
|
||||||
|
|
||||||
let result = query_discussions(&conn, &filters, &config)?;
|
let result = query_discussions(&conn, &filters, &config)?;
|
||||||
@@ -1122,6 +1222,8 @@ fn handle_discussions(
|
|||||||
start.elapsed().as_millis() as u64,
|
start.elapsed().as_millis() as u64,
|
||||||
args.fields.as_deref(),
|
args.fields.as_deref(),
|
||||||
robot_mode,
|
robot_mode,
|
||||||
|
effective_limit,
|
||||||
|
effective_include_notes,
|
||||||
),
|
),
|
||||||
"jsonl" => print_list_discussions_jsonl(&result),
|
"jsonl" => print_list_discussions_jsonl(&result),
|
||||||
"csv" => print_list_discussions_csv(&result),
|
"csv" => print_list_discussions_csv(&result),
|
||||||
@@ -1144,9 +1246,17 @@ pub fn print_list_discussions_json(
|
|||||||
elapsed_ms: u64,
|
elapsed_ms: u64,
|
||||||
fields: Option<&[String]>,
|
fields: Option<&[String]>,
|
||||||
robot_mode: bool,
|
robot_mode: bool,
|
||||||
|
effective_limit: usize,
|
||||||
|
effective_include_notes: usize,
|
||||||
) {
|
) {
|
||||||
let json_result = DiscussionListResultJson::from(result);
|
let json_result = DiscussionListResultJson::from(result);
|
||||||
let meta = RobotMeta { elapsed_ms };
|
let has_more = result.total_count as usize > json_result.showing;
|
||||||
|
let meta = serde_json::json!({
|
||||||
|
"elapsed_ms": elapsed_ms,
|
||||||
|
"effective_limit": effective_limit,
|
||||||
|
"effective_include_notes": effective_include_notes,
|
||||||
|
"has_more": has_more,
|
||||||
|
});
|
||||||
let output = serde_json::json!({
|
let output = serde_json::json!({
|
||||||
"ok": true,
|
"ok": true,
|
||||||
"data": json_result,
|
"data": json_result,
|
||||||
@@ -1325,7 +1435,7 @@ fn query_discussions_by_gitlab_id() {
|
|||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
#### Test 8: --include-notes populates notes array
|
#### Test 8: --include-notes populates notes array via batched query
|
||||||
|
|
||||||
```rust
|
```rust
|
||||||
#[test]
|
#[test]
|
||||||
@@ -1381,6 +1491,69 @@ fn discussions_bridge_fields_forced_in_robot_mode() {
|
|||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
|
#### Test 10: Output guardrails clamp limit and include_notes
|
||||||
|
|
||||||
|
```rust
|
||||||
|
#[test]
|
||||||
|
fn discussions_output_guardrails() {
|
||||||
|
// Verify that limit > 500 is clamped to 500
|
||||||
|
// Verify that include_notes > 20 is clamped to 20
|
||||||
|
// These are handler-level tests (not query-level)
|
||||||
|
assert_eq!(1000_usize.min(500), 500);
|
||||||
|
assert_eq!(50_usize.min(20), 20);
|
||||||
|
assert_eq!(5_usize.min(20), 5); // below cap stays unchanged
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Test 11: Ambiguity guardrail fires for cross-project discussion ID
|
||||||
|
|
||||||
|
```rust
|
||||||
|
#[test]
|
||||||
|
fn discussions_ambiguous_gitlab_discussion_id_across_projects() {
|
||||||
|
let conn = create_test_db();
|
||||||
|
insert_project(&conn, 1); // "group/repo-a"
|
||||||
|
insert_project(&conn, 2); // "group/repo-b"
|
||||||
|
// Insert discussions with same gitlab_discussion_id in different projects
|
||||||
|
insert_discussion(&conn, 1, "shared-id", 1, None, None, "Issue");
|
||||||
|
insert_discussion(&conn, 2, "shared-id", 2, None, None, "Issue");
|
||||||
|
|
||||||
|
let filters = DiscussionListFilters {
|
||||||
|
gitlab_discussion_id: Some("shared-id".to_string()),
|
||||||
|
project: None, // no project specified
|
||||||
|
..DiscussionListFilters::default()
|
||||||
|
};
|
||||||
|
let result = query_discussions(&conn, &filters, &Config::default());
|
||||||
|
assert!(result.is_err());
|
||||||
|
// Error should be Ambiguous with both project paths
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Test 12: has_more metadata is accurate
|
||||||
|
|
||||||
|
```rust
|
||||||
|
#[test]
|
||||||
|
fn discussions_has_more_metadata() {
|
||||||
|
let conn = create_test_db();
|
||||||
|
insert_project(&conn, 1);
|
||||||
|
insert_mr(&conn, 1, 1, 99, "Test MR");
|
||||||
|
// Insert 5 discussions
|
||||||
|
for i in 1..=5 {
|
||||||
|
insert_discussion(&conn, i, &format!("disc-{i}"), 1, None, Some(1), "MergeRequest");
|
||||||
|
insert_note_in_discussion(&conn, i, 500 + i, i, 1, "alice", "note");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Limit to 3 — should show has_more = true
|
||||||
|
let filters = DiscussionListFilters {
|
||||||
|
limit: 3,
|
||||||
|
..DiscussionListFilters::default_for_mr(99)
|
||||||
|
};
|
||||||
|
let result = query_discussions(&conn, &filters, &Config::default()).unwrap();
|
||||||
|
assert_eq!(result.discussions.len(), 3);
|
||||||
|
assert_eq!(result.total_count, 5);
|
||||||
|
// has_more = total_count > showing = 5 > 3 = true
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 4. Fix Robot-Docs Response Schemas
|
## 4. Fix Robot-Docs Response Schemas
|
||||||
@@ -1410,7 +1583,7 @@ Replace:
|
|||||||
With:
|
With:
|
||||||
```json
|
```json
|
||||||
"data": {
|
"data": {
|
||||||
"notes": "[{id:int, gitlab_id:int, author_username:string, body:string?, note_type:string?, is_system:bool, created_at_iso:string, updated_at_iso:string, position_new_path:string?, position_new_line:int?, position_old_path:string?, position_old_line:int?, resolvable:bool, resolved:bool, resolved_by:string?, noteable_type:string?, parent_iid:int?, parent_title:string?, project_path:string, gitlab_discussion_id:string}]",
|
"notes": "[{id:int, gitlab_id:int, gitlab_note_id:int, author_username:string, body:string?, note_type:string?, is_system:bool, created_at_iso:string, updated_at_iso:string, position_new_path:string?, position_new_line:int?, position_old_path:string?, position_old_line:int?, resolvable:bool, resolved:bool, resolved_by:string?, noteable_type:string?, parent_iid:int?, parent_title:string?, project_path:string, gitlab_discussion_id:string}]",
|
||||||
"total_count": "int",
|
"total_count": "int",
|
||||||
"showing": "int"
|
"showing": "int"
|
||||||
}
|
}
|
||||||
@@ -1442,11 +1615,11 @@ With:
|
|||||||
"response_schema": {
|
"response_schema": {
|
||||||
"ok": "bool",
|
"ok": "bool",
|
||||||
"data": {
|
"data": {
|
||||||
"discussions": "[{gitlab_discussion_id:string, noteable_type:string, parent_iid:int?, parent_title:string?, project_path:string, individual_note:bool, note_count:int, first_author:string?, first_note_body_snippet:string?, first_note_at_iso:string, last_note_at_iso:string, resolvable:bool, resolved:bool, position_new_path:string?, position_new_line:int?, notes:[NoteListRowJson]?}]",
|
"discussions": "[{gitlab_discussion_id:string, noteable_type:string, parent_iid:int?, parent_title:string?, project_path:string, individual_note:bool, note_count:int, first_author:string?, first_note_body_snippet:string?, first_note_at_iso:string, last_note_at_iso:string, resolvable:bool, resolved:bool, position_new_path:string?, position_new_line:int?, notes:[{...NoteListRowJson fields...}]?}]",
|
||||||
"total_count": "int",
|
"total_count": "int",
|
||||||
"showing": "int"
|
"showing": "int"
|
||||||
},
|
},
|
||||||
"meta": {"elapsed_ms": "int"}
|
"meta": {"elapsed_ms": "int", "effective_limit": "int", "effective_include_notes": "int", "has_more": "bool"}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
@@ -1473,33 +1646,83 @@ notes within show discussions now include `gitlab_note_id`.
|
|||||||
"discussions: Thread-level discussion listing with gitlab_discussion_id for API integration"
|
"discussions: Thread-level discussion listing with gitlab_discussion_id for API integration"
|
||||||
```
|
```
|
||||||
|
|
||||||
#### 4f. Add robot-docs contract tests
|
#### 4f. Add robot-docs contract tests (field-set parity)
|
||||||
|
|
||||||
**File**: `src/main.rs` (within `#[cfg(test)]` module)
|
**File**: `src/main.rs` (within `#[cfg(test)]` module)
|
||||||
|
|
||||||
Add lightweight tests that parse the robot-docs JSON output and assert required Bridge
|
Add tests that parse the robot-docs JSON output and compare declared fields against actual
|
||||||
Contract fields are present. This prevents schema drift — if someone adds a field to the
|
serialized struct fields. This is stronger than string-contains checks — it catches schema
|
||||||
struct but forgets to update robot-docs, the test fails.
|
drift in both directions (field added to struct but not docs, or field listed in docs but
|
||||||
|
removed from struct).
|
||||||
|
|
||||||
```rust
|
```rust
|
||||||
#[test]
|
/// Parse compact schema string "field1:type, field2:type?" into a set of field names
|
||||||
fn robot_docs_notes_schema_includes_bridge_fields() {
|
fn parse_schema_fields(schema: &str) -> HashSet<String> {
|
||||||
let docs = get_robot_docs_json(); // helper that builds the robot-docs Value
|
// Strip leading "[{" and trailing "}]", split on ", ", extract field name before ":"
|
||||||
let notes_schema = docs["commands"]["notes"]["response_schema"]["data"]["notes"]
|
schema.trim_start_matches("[{").trim_end_matches("}]")
|
||||||
.as_str().unwrap();
|
.split(", ")
|
||||||
assert!(notes_schema.contains("gitlab_discussion_id"));
|
.filter_map(|f| f.split(':').next())
|
||||||
assert!(notes_schema.contains("project_path"));
|
.map(|f| f.to_string())
|
||||||
assert!(notes_schema.contains("parent_iid"));
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Get the actual serialized field names from a sample JSON struct
|
||||||
|
fn sample_note_json_keys() -> HashSet<String> {
|
||||||
|
let row = NoteListRow { /* ... test defaults ... */ };
|
||||||
|
let json = NoteListRowJson::from(&row);
|
||||||
|
let value = serde_json::to_value(&json).unwrap();
|
||||||
|
value.as_object().unwrap().keys().cloned().collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn robot_docs_discussions_schema_includes_bridge_fields() {
|
fn robot_docs_notes_schema_matches_actual_fields() {
|
||||||
|
let docs = get_robot_docs_json();
|
||||||
|
let notes_schema = docs["commands"]["notes"]["response_schema"]["data"]["notes"]
|
||||||
|
.as_str().unwrap();
|
||||||
|
let declared = parse_schema_fields(notes_schema);
|
||||||
|
let actual = sample_note_json_keys();
|
||||||
|
|
||||||
|
// All bridge fields must be in both declared and actual
|
||||||
|
for bridge in &["gitlab_discussion_id", "project_path", "parent_iid", "gitlab_note_id"] {
|
||||||
|
assert!(declared.contains(*bridge), "robot-docs missing bridge field: {bridge}");
|
||||||
|
assert!(actual.contains(*bridge), "NoteListRowJson missing bridge field: {bridge}");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Every declared field should exist in the actual struct (no phantom docs)
|
||||||
|
for field in &declared {
|
||||||
|
assert!(actual.contains(field),
|
||||||
|
"robot-docs declares '{field}' but NoteListRowJson doesn't serialize it");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Every actual field should be declared in docs (no undocumented fields)
|
||||||
|
for field in &actual {
|
||||||
|
assert!(declared.contains(field),
|
||||||
|
"NoteListRowJson serializes '{field}' but robot-docs doesn't declare it");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn robot_docs_discussions_schema_matches_actual_fields() {
|
||||||
let docs = get_robot_docs_json();
|
let docs = get_robot_docs_json();
|
||||||
let disc_schema = docs["commands"]["discussions"]["response_schema"]["data"]["discussions"]
|
let disc_schema = docs["commands"]["discussions"]["response_schema"]["data"]["discussions"]
|
||||||
.as_str().unwrap();
|
.as_str().unwrap();
|
||||||
assert!(disc_schema.contains("gitlab_discussion_id"));
|
let declared = parse_schema_fields(disc_schema);
|
||||||
assert!(disc_schema.contains("project_path"));
|
let actual = sample_discussion_json_keys();
|
||||||
assert!(disc_schema.contains("parent_iid"));
|
|
||||||
|
for bridge in &["gitlab_discussion_id", "project_path", "parent_iid"] {
|
||||||
|
assert!(declared.contains(*bridge), "robot-docs missing bridge field: {bridge}");
|
||||||
|
assert!(actual.contains(*bridge), "DiscussionListRowJson missing bridge field: {bridge}");
|
||||||
|
}
|
||||||
|
|
||||||
|
for field in &declared {
|
||||||
|
assert!(actual.contains(field),
|
||||||
|
"robot-docs declares '{field}' but DiscussionListRowJson doesn't serialize it");
|
||||||
|
}
|
||||||
|
|
||||||
|
for field in &actual {
|
||||||
|
assert!(declared.contains(field),
|
||||||
|
"DiscussionListRowJson serializes '{field}' but robot-docs doesn't declare it");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@@ -1536,6 +1759,7 @@ fn notes_handler_json_includes_bridge_fields() {
|
|||||||
|
|
||||||
for note in value["notes"].as_array().unwrap() {
|
for note in value["notes"].as_array().unwrap() {
|
||||||
assert!(note.get("gitlab_discussion_id").is_some(), "missing gitlab_discussion_id");
|
assert!(note.get("gitlab_discussion_id").is_some(), "missing gitlab_discussion_id");
|
||||||
|
assert!(note.get("gitlab_note_id").is_some(), "missing gitlab_note_id");
|
||||||
assert!(note.get("project_path").is_some(), "missing project_path");
|
assert!(note.get("project_path").is_some(), "missing project_path");
|
||||||
assert!(note.get("parent_iid").is_some(), "missing parent_iid");
|
assert!(note.get("parent_iid").is_some(), "missing parent_iid");
|
||||||
}
|
}
|
||||||
@@ -1591,7 +1815,7 @@ once and reused by Changes 3 and 4.
|
|||||||
After all changes:
|
After all changes:
|
||||||
|
|
||||||
1. An agent can run `lore -J notes --for-mr 3929 --contains "really do prefer"` and get
|
1. An agent can run `lore -J notes --for-mr 3929 --contains "really do prefer"` and get
|
||||||
`gitlab_discussion_id` in the response
|
`gitlab_discussion_id` and `gitlab_note_id` in the response
|
||||||
2. An agent can run `lore -J discussions --for-mr 3929 --resolution unresolved` to see all
|
2. An agent can run `lore -J discussions --for-mr 3929 --resolution unresolved` to see all
|
||||||
open threads with their IDs
|
open threads with their IDs
|
||||||
3. An agent can run `lore -J mrs 3929` and see `gitlab_discussion_id`, `resolvable`,
|
3. An agent can run `lore -J mrs 3929` and see `gitlab_discussion_id`, `resolvable`,
|
||||||
@@ -1600,19 +1824,28 @@ After all changes:
|
|||||||
4. `lore robot-docs` lists actual field names for all commands
|
4. `lore robot-docs` lists actual field names for all commands
|
||||||
5. All existing tests still pass
|
5. All existing tests still pass
|
||||||
6. No clippy warnings (pedantic + nursery)
|
6. No clippy warnings (pedantic + nursery)
|
||||||
7. Robot-docs contract tests pass, preventing future schema drift
|
7. Robot-docs contract tests pass with field-set parity (not just string-contains), preventing
|
||||||
|
future schema drift in both directions
|
||||||
8. Bridge Contract fields (`project_path`, `noteable_type`, `parent_iid`,
|
8. Bridge Contract fields (`project_path`, `noteable_type`, `parent_iid`,
|
||||||
`gitlab_discussion_id`, `gitlab_note_id`) are present in every applicable read payload
|
`gitlab_discussion_id`, `gitlab_note_id`) are present in every applicable read payload
|
||||||
9. Bridge Contract fields survive `--fields` filtering in robot mode (guardrail enforced)
|
9. Bridge Contract fields survive `--fields` filtering in robot mode (guardrail enforced)
|
||||||
10. `--gitlab-discussion-id` filter works on both `notes` and `discussions` commands
|
10. `--gitlab-discussion-id` filter works on both `notes` and `discussions` commands
|
||||||
11. `--include-notes N` populates inline notes on `discussions` output
|
11. `--include-notes N` populates inline notes on `discussions` output via single batched query
|
||||||
12. CLI-level contract integration tests verify bridge fields through the full handler path
|
12. CLI-level contract integration tests verify bridge fields through the full handler path
|
||||||
|
13. `gitlab_note_id` is available in notes list output (alongside `gitlab_id` for back-compat)
|
||||||
|
and in show detail notes, providing a uniform field name across all commands
|
||||||
|
14. Ambiguity guardrail fires when `--gitlab-discussion-id` matches multiple projects without
|
||||||
|
`--project` specified
|
||||||
|
15. Output guardrails clamp `--limit` to 500 and `--include-notes` to 20; `meta` reports
|
||||||
|
effective values and `has_more` truncation flag
|
||||||
|
16. Discussion and show queries use deterministic ordering (COALESCE + id tiebreaker) to
|
||||||
|
prevent unstable output during partial sync states
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Rejected Recommendations
|
## Rejected Recommendations
|
||||||
|
|
||||||
- **Rename `id`→`note_id` and `gitlab_id`→`gitlab_note_id` in notes list output** — rejected because every existing consumer (agents, scripts, field presets) uses `id` and `gitlab_id`. The fields are unambiguous within the `notes` context. The show-command note structs are a different story (they have no IDs at all), so we add `gitlab_note_id` there where it's genuinely missing. Renaming established fields is churn without proportional benefit.
|
- **Rename `id`→`note_id` and `gitlab_id`→`gitlab_note_id` in notes list output** — rejected because every existing consumer (agents, scripts, field presets) uses `id` and `gitlab_id`. The fields are unambiguous within the `notes` context. The show-command note structs are a different story (they have no IDs at all), so we add `gitlab_note_id` there where it's genuinely missing. Renaming established fields is churn without proportional benefit. (Updated: we now ADD `gitlab_note_id` as an alias alongside `gitlab_id` per iteration 3 feedback.)
|
||||||
- **Keyset cursor-based pagination (`--cursor` flag)** — rejected because no existing lore command has pagination, agents use `--limit` effectively, and adding a cursor mechanism is significant scope creep. Tracked as potential future work if agents hit real pagination needs.
|
- **Keyset cursor-based pagination (`--cursor` flag)** — rejected because no existing lore command has pagination, agents use `--limit` effectively, and adding a cursor mechanism is significant scope creep. Tracked as potential future work if agents hit real pagination needs.
|
||||||
- **Split `note_count` into `user_note_count`/`total_note_count` and rename `first_author` to `first_user_author`** — rejected because `note_count` already excludes system notes by query design (the `WHERE is_system = 0` / `CASE WHEN` filter), and `first_author` already targets the first non-system note. The current naming is clear and consistent with how `notes --include-system` works elsewhere.
|
- **Split `note_count` into `user_note_count`/`total_note_count` and rename `first_author` to `first_user_author`** — rejected because `note_count` already excludes system notes by query design (the `WHERE is_system = 0` / `CASE WHEN` filter), and `first_author` already targets the first non-system note. The current naming is clear and consistent with how `notes --include-system` works elsewhere.
|
||||||
- **Match path filter on both `position_new_path` and `position_old_path`** — rejected because agents care about where code is *now* (new path), not where it was before a rename. Matching old paths adds complexity and returns confusing results for moved files.
|
- **Match path filter on both `position_new_path` and `position_old_path`** — rejected because agents care about where code is *now* (new path), not where it was before a rename. Matching old paths adds complexity and returns confusing results for moved files.
|
||||||
@@ -1621,3 +1854,6 @@ After all changes:
|
|||||||
- **Structured robot-docs schema (JSON objects instead of string blobs)** — rejected because the current compact string format is intentionally token-efficient for agent consumption. Switching to nested JSON objects per field would significantly bloat robot-docs output. The string-based contract tests are sufficient — they test what agents actually parse. Agents already work with the inline field listing format used by `issues` and `mrs`.
|
- **Structured robot-docs schema (JSON objects instead of string blobs)** — rejected because the current compact string format is intentionally token-efficient for agent consumption. Switching to nested JSON objects per field would significantly bloat robot-docs output. The string-based contract tests are sufficient — they test what agents actually parse. Agents already work with the inline field listing format used by `issues` and `mrs`.
|
||||||
- **`bridge_contract` meta-section in robot-docs output** — rejected because agents don't need a separate meta-contract section; they need correct field listings per command, which we already provide. Adding a cross-cutting contract section to robot-docs adds documentation surface area without improving the agent workflow.
|
- **`bridge_contract` meta-section in robot-docs output** — rejected because agents don't need a separate meta-contract section; they need correct field listings per command, which we already provide. Adding a cross-cutting contract section to robot-docs adds documentation surface area without improving the agent workflow.
|
||||||
- **Performance regression benchmark test (ignored by default)** — rejected because timing-based assertions are inherently flaky across machines, CI environments, and load conditions. Performance is validated through query plan analysis (EXPLAIN) and manual profiling, not hard-coded elapsed-time thresholds.
|
- **Performance regression benchmark test (ignored by default)** — rejected because timing-based assertions are inherently flaky across machines, CI environments, and load conditions. Performance is validated through query plan analysis (EXPLAIN) and manual profiling, not hard-coded elapsed-time thresholds.
|
||||||
|
- **Make `--discussion-id` and `--gitlab-discussion-id` aliases for the same backing filter** — rejected because they filter on different identifiers: `--discussion-id` takes the internal integer ID (existing behavior), while `--gitlab-discussion-id` takes the external string ID. These serve fundamentally different use cases (internal cross-referencing vs. external API bridging) and cannot be collapsed without breaking existing consumers.
|
||||||
|
- **`--with-write-hints` flag for inline glab endpoint templates** — rejected because this couples lore's read surface to glab's API surface, violating the read/write split principle. The Bridge Contract gives agents the raw identifiers; constructing glab commands is the agent's responsibility. Adding endpoint templates would require lore to track glab API changes, creating an unnecessary maintenance burden.
|
||||||
|
- **Show-command note ordering change (`ORDER BY COALESCE(position, ...), created_at, id`)** — rejected because show-command note ordering within a discussion thread is out of scope for this plan. The existing ordering works correctly for present data; the defensive COALESCE pattern is applied to discussion-level ordering where it matters for agent workflows.
|
||||||
|
|||||||
@@ -243,6 +243,15 @@ const COMMAND_FLAGS: &[(&str, &[&str])] = &[
|
|||||||
"--limit",
|
"--limit",
|
||||||
],
|
],
|
||||||
),
|
),
|
||||||
|
(
|
||||||
|
"trace",
|
||||||
|
&[
|
||||||
|
"--project",
|
||||||
|
"--discussions",
|
||||||
|
"--no-follow-renames",
|
||||||
|
"--limit",
|
||||||
|
],
|
||||||
|
),
|
||||||
("generate-docs", &["--full", "--project"]),
|
("generate-docs", &["--full", "--project"]),
|
||||||
("completions", &[]),
|
("completions", &[]),
|
||||||
("robot-docs", &["--brief"]),
|
("robot-docs", &["--brief"]),
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ pub mod stats;
|
|||||||
pub mod sync;
|
pub mod sync;
|
||||||
pub mod sync_status;
|
pub mod sync_status;
|
||||||
pub mod timeline;
|
pub mod timeline;
|
||||||
|
pub mod trace;
|
||||||
pub mod who;
|
pub mod who;
|
||||||
|
|
||||||
pub use auth_test::run_auth_test;
|
pub use auth_test::run_auth_test;
|
||||||
@@ -48,4 +49,5 @@ pub use stats::{print_stats, print_stats_json, run_stats};
|
|||||||
pub use sync::{SyncOptions, SyncResult, print_sync, print_sync_json, run_sync};
|
pub use sync::{SyncOptions, SyncResult, print_sync, print_sync_json, run_sync};
|
||||||
pub use sync_status::{print_sync_status, print_sync_status_json, run_sync_status};
|
pub use sync_status::{print_sync_status, print_sync_status_json, run_sync_status};
|
||||||
pub use timeline::{TimelineParams, print_timeline, print_timeline_json_with_meta, run_timeline};
|
pub use timeline::{TimelineParams, print_timeline, print_timeline_json_with_meta, run_timeline};
|
||||||
|
pub use trace::{parse_trace_path, print_trace, print_trace_json};
|
||||||
pub use who::{WhoRun, print_who_human, print_who_json, run_who};
|
pub use who::{WhoRun, print_who_human, print_who_json, run_who};
|
||||||
|
|||||||
196
src/cli/commands/trace.rs
Normal file
196
src/cli/commands/trace.rs
Normal file
@@ -0,0 +1,196 @@
|
|||||||
|
use crate::cli::render::{self, Icons, Theme};
|
||||||
|
use crate::core::trace::{TraceChain, TraceResult};
|
||||||
|
|
||||||
|
/// Parse a path with optional `:line` suffix.
|
||||||
|
///
|
||||||
|
/// Handles Windows drive letters (e.g. `C:/foo.rs`) by checking that the
|
||||||
|
/// prefix before the colon is not a single ASCII letter.
|
||||||
|
pub fn parse_trace_path(input: &str) -> (String, Option<u32>) {
|
||||||
|
if let Some((path, suffix)) = input.rsplit_once(':')
|
||||||
|
&& !path.is_empty()
|
||||||
|
&& let Ok(line) = suffix.parse::<u32>()
|
||||||
|
// Reject Windows drive letters: single ASCII letter before colon
|
||||||
|
&& (path.len() > 1 || !path.chars().next().unwrap_or(' ').is_ascii_alphabetic())
|
||||||
|
{
|
||||||
|
return (path.to_string(), Some(line));
|
||||||
|
}
|
||||||
|
(input.to_string(), None)
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── Human output ────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
pub fn print_trace(result: &TraceResult) {
|
||||||
|
let chain_info = if result.total_chains == 1 {
|
||||||
|
"1 chain".to_string()
|
||||||
|
} else {
|
||||||
|
format!("{} chains", result.total_chains)
|
||||||
|
};
|
||||||
|
|
||||||
|
let paths_info = if result.resolved_paths.len() > 1 {
|
||||||
|
format!(", {} paths", result.resolved_paths.len())
|
||||||
|
} else {
|
||||||
|
String::new()
|
||||||
|
};
|
||||||
|
|
||||||
|
println!();
|
||||||
|
println!(
|
||||||
|
"{}",
|
||||||
|
Theme::bold().render(&format!(
|
||||||
|
"Trace: {} ({}{})",
|
||||||
|
result.path, chain_info, paths_info
|
||||||
|
))
|
||||||
|
);
|
||||||
|
|
||||||
|
// Rename chain
|
||||||
|
if result.renames_followed && result.resolved_paths.len() > 1 {
|
||||||
|
let chain_str: Vec<&str> = result.resolved_paths.iter().map(String::as_str).collect();
|
||||||
|
println!(
|
||||||
|
" Rename chain: {}",
|
||||||
|
Theme::dim().render(&chain_str.join(" -> "))
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
if result.trace_chains.is_empty() {
|
||||||
|
println!(
|
||||||
|
"\n {} {}",
|
||||||
|
Icons::info(),
|
||||||
|
Theme::dim().render("No trace chains found for this file.")
|
||||||
|
);
|
||||||
|
println!(
|
||||||
|
" {}",
|
||||||
|
Theme::dim()
|
||||||
|
.render("Hint: Run 'lore sync' to fetch MR file changes and cross-references.")
|
||||||
|
);
|
||||||
|
println!();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
println!();
|
||||||
|
|
||||||
|
for chain in &result.trace_chains {
|
||||||
|
print_chain(chain);
|
||||||
|
}
|
||||||
|
|
||||||
|
println!();
|
||||||
|
}
|
||||||
|
|
||||||
|
fn print_chain(chain: &TraceChain) {
|
||||||
|
let (icon, state_style) = match chain.mr_state.as_str() {
|
||||||
|
"merged" => (Icons::mr_merged(), Theme::accent()),
|
||||||
|
"opened" => (Icons::mr_opened(), Theme::success()),
|
||||||
|
"closed" => (Icons::mr_closed(), Theme::warning()),
|
||||||
|
_ => (Icons::mr_opened(), Theme::dim()),
|
||||||
|
};
|
||||||
|
|
||||||
|
let date = chain
|
||||||
|
.merged_at_iso
|
||||||
|
.as_deref()
|
||||||
|
.or(Some(chain.updated_at_iso.as_str()))
|
||||||
|
.unwrap_or("")
|
||||||
|
.split('T')
|
||||||
|
.next()
|
||||||
|
.unwrap_or("");
|
||||||
|
|
||||||
|
println!(
|
||||||
|
" {} {} {} {} @{} {} {}",
|
||||||
|
icon,
|
||||||
|
Theme::accent().render(&format!("!{}", chain.mr_iid)),
|
||||||
|
render::truncate(&chain.mr_title, 50),
|
||||||
|
state_style.render(&chain.mr_state),
|
||||||
|
chain.mr_author,
|
||||||
|
date,
|
||||||
|
Theme::dim().render(&chain.change_type),
|
||||||
|
);
|
||||||
|
|
||||||
|
// Linked issues
|
||||||
|
for issue in &chain.issues {
|
||||||
|
let ref_icon = match issue.reference_type.as_str() {
|
||||||
|
"closes" => Icons::issue_closed(),
|
||||||
|
_ => Icons::issue_opened(),
|
||||||
|
};
|
||||||
|
|
||||||
|
println!(
|
||||||
|
" {} #{} {} {} [{}]",
|
||||||
|
ref_icon,
|
||||||
|
issue.iid,
|
||||||
|
render::truncate(&issue.title, 45),
|
||||||
|
Theme::dim().render(&issue.state),
|
||||||
|
Theme::dim().render(&issue.reference_type),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Discussions
|
||||||
|
for disc in &chain.discussions {
|
||||||
|
let date = disc.created_at_iso.split('T').next().unwrap_or("");
|
||||||
|
println!(
|
||||||
|
" {} @{} ({}) [{}]: {}",
|
||||||
|
Icons::note(),
|
||||||
|
disc.author_username,
|
||||||
|
date,
|
||||||
|
Theme::dim().render(&disc.path),
|
||||||
|
disc.body_snippet
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── Robot (JSON) output ─────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
pub fn print_trace_json(result: &TraceResult, elapsed_ms: u64, line_requested: Option<u32>) {
|
||||||
|
let output = serde_json::json!({
|
||||||
|
"ok": true,
|
||||||
|
"data": {
|
||||||
|
"path": result.path,
|
||||||
|
"resolved_paths": result.resolved_paths,
|
||||||
|
"trace_chains": result.trace_chains,
|
||||||
|
},
|
||||||
|
"meta": {
|
||||||
|
"tier": "api_only",
|
||||||
|
"line_requested": line_requested,
|
||||||
|
"elapsed_ms": elapsed_ms,
|
||||||
|
"total_chains": result.total_chains,
|
||||||
|
"renames_followed": result.renames_followed,
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
println!("{}", serde_json::to_string(&output).unwrap_or_default());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_parse_trace_path_simple() {
|
||||||
|
let (path, line) = parse_trace_path("src/foo.rs");
|
||||||
|
assert_eq!(path, "src/foo.rs");
|
||||||
|
assert_eq!(line, None);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_parse_trace_path_with_line() {
|
||||||
|
let (path, line) = parse_trace_path("src/foo.rs:42");
|
||||||
|
assert_eq!(path, "src/foo.rs");
|
||||||
|
assert_eq!(line, Some(42));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_parse_trace_path_windows() {
|
||||||
|
let (path, line) = parse_trace_path("C:/foo.rs");
|
||||||
|
assert_eq!(path, "C:/foo.rs");
|
||||||
|
assert_eq!(line, None);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_parse_trace_path_directory() {
|
||||||
|
let (path, line) = parse_trace_path("src/auth/");
|
||||||
|
assert_eq!(path, "src/auth/");
|
||||||
|
assert_eq!(line, None);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_parse_trace_path_with_line_zero() {
|
||||||
|
let (path, line) = parse_trace_path("file.rs:0");
|
||||||
|
assert_eq!(path, "file.rs");
|
||||||
|
assert_eq!(line, Some(0));
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -238,6 +238,9 @@ pub enum Commands {
|
|||||||
#[command(name = "file-history")]
|
#[command(name = "file-history")]
|
||||||
FileHistory(FileHistoryArgs),
|
FileHistory(FileHistoryArgs),
|
||||||
|
|
||||||
|
/// Trace why code was introduced: file -> MR -> issue -> discussion
|
||||||
|
Trace(TraceArgs),
|
||||||
|
|
||||||
/// Detect discussion divergence from original intent
|
/// Detect discussion divergence from original intent
|
||||||
Drift {
|
Drift {
|
||||||
/// Entity type (currently only "issues" supported)
|
/// Entity type (currently only "issues" supported)
|
||||||
@@ -1006,6 +1009,38 @@ pub struct FileHistoryArgs {
|
|||||||
pub limit: usize,
|
pub limit: usize,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Parser)]
|
||||||
|
#[command(after_help = "\x1b[1mExamples:\x1b[0m
|
||||||
|
lore trace src/main.rs # Why was this file changed?
|
||||||
|
lore trace src/auth/ -p group/repo # Scoped to project
|
||||||
|
lore trace src/foo.rs --discussions # Include DiffNote context
|
||||||
|
lore trace src/bar.rs:42 # Line hint (Tier 2 warning)")]
|
||||||
|
pub struct TraceArgs {
|
||||||
|
/// File path to trace (supports :line suffix for future Tier 2)
|
||||||
|
pub path: String,
|
||||||
|
|
||||||
|
/// Scope to a specific project (fuzzy match)
|
||||||
|
#[arg(short = 'p', long, help_heading = "Filters")]
|
||||||
|
pub project: Option<String>,
|
||||||
|
|
||||||
|
/// Include DiffNote discussion snippets
|
||||||
|
#[arg(long, help_heading = "Output")]
|
||||||
|
pub discussions: bool,
|
||||||
|
|
||||||
|
/// Disable rename chain resolution
|
||||||
|
#[arg(long = "no-follow-renames", help_heading = "Filters")]
|
||||||
|
pub no_follow_renames: bool,
|
||||||
|
|
||||||
|
/// Maximum trace chains to display
|
||||||
|
#[arg(
|
||||||
|
short = 'n',
|
||||||
|
long = "limit",
|
||||||
|
default_value = "20",
|
||||||
|
help_heading = "Output"
|
||||||
|
)]
|
||||||
|
pub limit: usize,
|
||||||
|
}
|
||||||
|
|
||||||
#[derive(Parser)]
|
#[derive(Parser)]
|
||||||
pub struct CountArgs {
|
pub struct CountArgs {
|
||||||
/// Entity type to count (issues, mrs, discussions, notes, events)
|
/// Entity type to count (issues, mrs, discussions, notes, events)
|
||||||
|
|||||||
@@ -21,6 +21,7 @@ pub mod timeline;
|
|||||||
pub mod timeline_collect;
|
pub mod timeline_collect;
|
||||||
pub mod timeline_expand;
|
pub mod timeline_expand;
|
||||||
pub mod timeline_seed;
|
pub mod timeline_seed;
|
||||||
|
pub mod trace;
|
||||||
|
|
||||||
pub use config::Config;
|
pub use config::Config;
|
||||||
pub use error::{LoreError, Result};
|
pub use error::{LoreError, Result};
|
||||||
|
|||||||
283
src/core/trace.rs
Normal file
283
src/core/trace.rs
Normal file
@@ -0,0 +1,283 @@
|
|||||||
|
use serde::Serialize;
|
||||||
|
|
||||||
|
use super::error::Result;
|
||||||
|
use super::file_history::resolve_rename_chain;
|
||||||
|
use super::time::ms_to_iso;
|
||||||
|
|
||||||
|
/// Maximum rename chain BFS depth.
|
||||||
|
const MAX_RENAME_HOPS: usize = 10;
|
||||||
|
|
||||||
|
/// A linked issue found via entity_references on the MR.
|
||||||
|
#[derive(Debug, Serialize)]
|
||||||
|
pub struct TraceIssue {
|
||||||
|
pub iid: i64,
|
||||||
|
pub title: String,
|
||||||
|
pub state: String,
|
||||||
|
pub reference_type: String,
|
||||||
|
pub web_url: Option<String>,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A DiffNote discussion snippet relevant to the traced file.
|
||||||
|
#[derive(Debug, Serialize)]
|
||||||
|
pub struct TraceDiscussion {
|
||||||
|
pub discussion_id: String,
|
||||||
|
pub mr_iid: i64,
|
||||||
|
pub author_username: String,
|
||||||
|
pub body_snippet: String,
|
||||||
|
pub path: String,
|
||||||
|
pub created_at_iso: String,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A single trace chain: an MR that touched the file, plus linked issues and discussions.
|
||||||
|
#[derive(Debug, Serialize)]
|
||||||
|
pub struct TraceChain {
|
||||||
|
pub mr_iid: i64,
|
||||||
|
pub mr_title: String,
|
||||||
|
pub mr_state: String,
|
||||||
|
pub mr_author: String,
|
||||||
|
pub change_type: String,
|
||||||
|
pub merged_at_iso: Option<String>,
|
||||||
|
pub updated_at_iso: String,
|
||||||
|
pub web_url: Option<String>,
|
||||||
|
pub issues: Vec<TraceIssue>,
|
||||||
|
pub discussions: Vec<TraceDiscussion>,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Result of a trace query.
|
||||||
|
#[derive(Debug, Serialize)]
|
||||||
|
pub struct TraceResult {
|
||||||
|
pub path: String,
|
||||||
|
pub resolved_paths: Vec<String>,
|
||||||
|
pub renames_followed: bool,
|
||||||
|
pub trace_chains: Vec<TraceChain>,
|
||||||
|
pub total_chains: usize,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Tier of the trace result.
|
||||||
|
#[derive(Debug, Clone, Copy, Serialize)]
|
||||||
|
pub enum TraceTier {
|
||||||
|
/// API-only: file -> MR -> issue via local DB
|
||||||
|
ApiOnly,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl TraceTier {
|
||||||
|
pub fn as_str(self) -> &'static str {
|
||||||
|
match self {
|
||||||
|
Self::ApiOnly => "api_only",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Run the trace query: file -> MR -> issue chain.
|
||||||
|
pub fn run_trace(
|
||||||
|
conn: &rusqlite::Connection,
|
||||||
|
project_id: Option<i64>,
|
||||||
|
path: &str,
|
||||||
|
follow_renames: bool,
|
||||||
|
include_discussions: bool,
|
||||||
|
limit: usize,
|
||||||
|
) -> Result<TraceResult> {
|
||||||
|
// Resolve rename chain
|
||||||
|
let (all_paths, renames_followed) = if follow_renames {
|
||||||
|
if let Some(pid) = project_id {
|
||||||
|
let chain = resolve_rename_chain(conn, pid, path, MAX_RENAME_HOPS)?;
|
||||||
|
let followed = chain.len() > 1;
|
||||||
|
(chain, followed)
|
||||||
|
} else {
|
||||||
|
(vec![path.to_string()], false)
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
(vec![path.to_string()], false)
|
||||||
|
};
|
||||||
|
|
||||||
|
// Build placeholders for IN clause
|
||||||
|
let placeholders: Vec<String> = (0..all_paths.len())
|
||||||
|
.map(|i| format!("?{}", i + 2))
|
||||||
|
.collect();
|
||||||
|
let in_clause = placeholders.join(", ");
|
||||||
|
|
||||||
|
let project_filter = if project_id.is_some() {
|
||||||
|
"AND mfc.project_id = ?1"
|
||||||
|
} else {
|
||||||
|
""
|
||||||
|
};
|
||||||
|
|
||||||
|
// Step 1: Find MRs that touched the file
|
||||||
|
let mr_sql = format!(
|
||||||
|
"SELECT DISTINCT \
|
||||||
|
mr.id, mr.iid, mr.title, mr.state, mr.author_username, \
|
||||||
|
mfc.change_type, mr.merged_at, mr.updated_at, mr.web_url \
|
||||||
|
FROM mr_file_changes mfc \
|
||||||
|
JOIN merge_requests mr ON mr.id = mfc.merge_request_id \
|
||||||
|
WHERE mfc.new_path IN ({in_clause}) {project_filter} \
|
||||||
|
ORDER BY COALESCE(mr.merged_at, mr.updated_at) DESC \
|
||||||
|
LIMIT ?{}",
|
||||||
|
all_paths.len() + 2
|
||||||
|
);
|
||||||
|
|
||||||
|
let mut stmt = conn.prepare(&mr_sql)?;
|
||||||
|
|
||||||
|
let mut params: Vec<Box<dyn rusqlite::types::ToSql>> = Vec::new();
|
||||||
|
params.push(Box::new(project_id.unwrap_or(0)));
|
||||||
|
for p in &all_paths {
|
||||||
|
params.push(Box::new(p.clone()));
|
||||||
|
}
|
||||||
|
params.push(Box::new(limit as i64));
|
||||||
|
|
||||||
|
let param_refs: Vec<&dyn rusqlite::types::ToSql> = params.iter().map(|p| p.as_ref()).collect();
|
||||||
|
|
||||||
|
struct MrRow {
|
||||||
|
id: i64,
|
||||||
|
iid: i64,
|
||||||
|
title: String,
|
||||||
|
state: String,
|
||||||
|
author: String,
|
||||||
|
change_type: String,
|
||||||
|
merged_at: Option<i64>,
|
||||||
|
updated_at: i64,
|
||||||
|
web_url: Option<String>,
|
||||||
|
}
|
||||||
|
|
||||||
|
let mr_rows: Vec<MrRow> = stmt
|
||||||
|
.query_map(param_refs.as_slice(), |row| {
|
||||||
|
Ok(MrRow {
|
||||||
|
id: row.get(0)?,
|
||||||
|
iid: row.get(1)?,
|
||||||
|
title: row.get(2)?,
|
||||||
|
state: row.get(3)?,
|
||||||
|
author: row.get(4)?,
|
||||||
|
change_type: row.get(5)?,
|
||||||
|
merged_at: row.get(6)?,
|
||||||
|
updated_at: row.get(7)?,
|
||||||
|
web_url: row.get(8)?,
|
||||||
|
})
|
||||||
|
})?
|
||||||
|
.filter_map(std::result::Result::ok)
|
||||||
|
.collect();
|
||||||
|
|
||||||
|
// Step 2: For each MR, find linked issues + optional discussions
|
||||||
|
let mut trace_chains = Vec::with_capacity(mr_rows.len());
|
||||||
|
|
||||||
|
for mr in &mr_rows {
|
||||||
|
let issues = fetch_linked_issues(conn, mr.id)?;
|
||||||
|
|
||||||
|
let discussions = if include_discussions {
|
||||||
|
fetch_trace_discussions(conn, mr.id, mr.iid, &all_paths)?
|
||||||
|
} else {
|
||||||
|
Vec::new()
|
||||||
|
};
|
||||||
|
|
||||||
|
trace_chains.push(TraceChain {
|
||||||
|
mr_iid: mr.iid,
|
||||||
|
mr_title: mr.title.clone(),
|
||||||
|
mr_state: mr.state.clone(),
|
||||||
|
mr_author: mr.author.clone(),
|
||||||
|
change_type: mr.change_type.clone(),
|
||||||
|
merged_at_iso: mr.merged_at.map(ms_to_iso),
|
||||||
|
updated_at_iso: ms_to_iso(mr.updated_at),
|
||||||
|
web_url: mr.web_url.clone(),
|
||||||
|
issues,
|
||||||
|
discussions,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
let total_chains = trace_chains.len();
|
||||||
|
|
||||||
|
Ok(TraceResult {
|
||||||
|
path: path.to_string(),
|
||||||
|
resolved_paths: all_paths,
|
||||||
|
renames_followed,
|
||||||
|
trace_chains,
|
||||||
|
total_chains,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Fetch issues linked to an MR via entity_references.
|
||||||
|
/// source = merge_request -> target = issue (closes/mentioned/related)
|
||||||
|
fn fetch_linked_issues(conn: &rusqlite::Connection, mr_id: i64) -> Result<Vec<TraceIssue>> {
|
||||||
|
let sql = "SELECT DISTINCT i.iid, i.title, i.state, er.reference_type, i.web_url \
|
||||||
|
FROM entity_references er \
|
||||||
|
JOIN issues i ON i.id = er.target_entity_id \
|
||||||
|
WHERE er.source_entity_type = 'merge_request' \
|
||||||
|
AND er.source_entity_id = ?1 \
|
||||||
|
AND er.target_entity_type = 'issue' \
|
||||||
|
AND er.target_entity_id IS NOT NULL \
|
||||||
|
ORDER BY \
|
||||||
|
CASE er.reference_type WHEN 'closes' THEN 0 WHEN 'related' THEN 1 ELSE 2 END, \
|
||||||
|
i.iid";
|
||||||
|
|
||||||
|
let mut stmt = conn.prepare(sql)?;
|
||||||
|
let issues: Vec<TraceIssue> = stmt
|
||||||
|
.query_map(rusqlite::params![mr_id], |row| {
|
||||||
|
Ok(TraceIssue {
|
||||||
|
iid: row.get(0)?,
|
||||||
|
title: row.get(1)?,
|
||||||
|
state: row.get(2)?,
|
||||||
|
reference_type: row.get(3)?,
|
||||||
|
web_url: row.get(4)?,
|
||||||
|
})
|
||||||
|
})?
|
||||||
|
.filter_map(std::result::Result::ok)
|
||||||
|
.collect();
|
||||||
|
|
||||||
|
Ok(issues)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Fetch DiffNote discussions on a specific MR that reference the traced paths.
|
||||||
|
fn fetch_trace_discussions(
|
||||||
|
conn: &rusqlite::Connection,
|
||||||
|
mr_id: i64,
|
||||||
|
mr_iid: i64,
|
||||||
|
paths: &[String],
|
||||||
|
) -> Result<Vec<TraceDiscussion>> {
|
||||||
|
let placeholders: Vec<String> = (0..paths.len()).map(|i| format!("?{}", i + 2)).collect();
|
||||||
|
let in_clause = placeholders.join(", ");
|
||||||
|
|
||||||
|
let sql = format!(
|
||||||
|
"SELECT d.gitlab_discussion_id, n.author_username, n.body, n.position_new_path, n.created_at \
|
||||||
|
FROM notes n \
|
||||||
|
JOIN discussions d ON d.id = n.discussion_id \
|
||||||
|
WHERE d.merge_request_id = ?1 \
|
||||||
|
AND n.position_new_path IN ({in_clause}) \
|
||||||
|
AND n.is_system = 0 \
|
||||||
|
ORDER BY n.created_at DESC \
|
||||||
|
LIMIT 20"
|
||||||
|
);
|
||||||
|
|
||||||
|
let mut stmt = conn.prepare(&sql)?;
|
||||||
|
|
||||||
|
let mut params: Vec<Box<dyn rusqlite::types::ToSql>> = Vec::new();
|
||||||
|
params.push(Box::new(mr_id));
|
||||||
|
for p in paths {
|
||||||
|
params.push(Box::new(p.clone()));
|
||||||
|
}
|
||||||
|
|
||||||
|
let param_refs: Vec<&dyn rusqlite::types::ToSql> = params.iter().map(|p| p.as_ref()).collect();
|
||||||
|
|
||||||
|
let discussions: Vec<TraceDiscussion> = stmt
|
||||||
|
.query_map(param_refs.as_slice(), |row| {
|
||||||
|
let body: String = row.get(2)?;
|
||||||
|
let snippet = if body.len() > 200 {
|
||||||
|
format!("{}...", &body[..body.floor_char_boundary(200)])
|
||||||
|
} else {
|
||||||
|
body
|
||||||
|
};
|
||||||
|
let created_at: i64 = row.get(4)?;
|
||||||
|
Ok(TraceDiscussion {
|
||||||
|
discussion_id: row.get(0)?,
|
||||||
|
mr_iid,
|
||||||
|
author_username: row.get(1)?,
|
||||||
|
body_snippet: snippet,
|
||||||
|
path: row.get(3)?,
|
||||||
|
created_at_iso: ms_to_iso(created_at),
|
||||||
|
})
|
||||||
|
})?
|
||||||
|
.filter_map(std::result::Result::ok)
|
||||||
|
.collect();
|
||||||
|
|
||||||
|
Ok(discussions)
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
#[path = "trace_tests.rs"]
|
||||||
|
mod tests;
|
||||||
260
src/core/trace_tests.rs
Normal file
260
src/core/trace_tests.rs
Normal file
@@ -0,0 +1,260 @@
|
|||||||
|
use super::*;
|
||||||
|
use crate::core::db::{create_connection, run_migrations};
|
||||||
|
use std::path::Path;
|
||||||
|
|
||||||
|
fn setup_test_db() -> rusqlite::Connection {
|
||||||
|
let conn = create_connection(Path::new(":memory:")).unwrap();
|
||||||
|
run_migrations(&conn).unwrap();
|
||||||
|
conn
|
||||||
|
}
|
||||||
|
|
||||||
|
fn seed_project(conn: &rusqlite::Connection) -> i64 {
|
||||||
|
conn.execute(
|
||||||
|
"INSERT INTO projects (id, gitlab_project_id, path_with_namespace, web_url, created_at, updated_at)
|
||||||
|
VALUES (1, 100, 'group/repo', 'https://gitlab.example.com/group/repo', 1000, 2000)",
|
||||||
|
[],
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
1
|
||||||
|
}
|
||||||
|
|
||||||
|
fn insert_mr(
|
||||||
|
conn: &rusqlite::Connection,
|
||||||
|
id: i64,
|
||||||
|
iid: i64,
|
||||||
|
title: &str,
|
||||||
|
state: &str,
|
||||||
|
merged_at: Option<i64>,
|
||||||
|
) {
|
||||||
|
conn.execute(
|
||||||
|
"INSERT INTO merge_requests (id, gitlab_id, iid, project_id, title, state, author_username, \
|
||||||
|
created_at, updated_at, last_seen_at, source_branch, target_branch, merged_at, web_url)
|
||||||
|
VALUES (?1, ?2, ?3, 1, ?4, ?5, 'dev', 1000, 2000, 2000, 'feature', 'main', ?6, \
|
||||||
|
'https://gitlab.example.com/group/repo/-/merge_requests/' || ?3)",
|
||||||
|
rusqlite::params![id, 300 + id, iid, title, state, merged_at],
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
fn insert_file_change(
|
||||||
|
conn: &rusqlite::Connection,
|
||||||
|
mr_id: i64,
|
||||||
|
old_path: Option<&str>,
|
||||||
|
new_path: &str,
|
||||||
|
change_type: &str,
|
||||||
|
) {
|
||||||
|
conn.execute(
|
||||||
|
"INSERT INTO mr_file_changes (merge_request_id, project_id, old_path, new_path, change_type)
|
||||||
|
VALUES (?1, 1, ?2, ?3, ?4)",
|
||||||
|
rusqlite::params![mr_id, old_path, new_path, change_type],
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
fn insert_entity_ref(
|
||||||
|
conn: &rusqlite::Connection,
|
||||||
|
source_type: &str,
|
||||||
|
source_id: i64,
|
||||||
|
target_type: &str,
|
||||||
|
target_id: i64,
|
||||||
|
ref_type: &str,
|
||||||
|
) {
|
||||||
|
conn.execute(
|
||||||
|
"INSERT INTO entity_references (project_id, source_entity_type, source_entity_id, \
|
||||||
|
target_entity_type, target_entity_id, reference_type, source_method, created_at)
|
||||||
|
VALUES (1, ?1, ?2, ?3, ?4, ?5, 'api', 1000)",
|
||||||
|
rusqlite::params![source_type, source_id, target_type, target_id, ref_type],
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
fn insert_issue(conn: &rusqlite::Connection, id: i64, iid: i64, title: &str, state: &str) {
|
||||||
|
conn.execute(
|
||||||
|
"INSERT INTO issues (id, gitlab_id, project_id, iid, title, state, created_at, updated_at, \
|
||||||
|
last_seen_at, web_url)
|
||||||
|
VALUES (?1, ?2, 1, ?3, ?4, ?5, 1000, 2000, 2000, \
|
||||||
|
'https://gitlab.example.com/group/repo/-/issues/' || ?3)",
|
||||||
|
rusqlite::params![id, 400 + id, iid, title, state],
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
fn insert_discussion_and_note(
|
||||||
|
conn: &rusqlite::Connection,
|
||||||
|
discussion_id: i64,
|
||||||
|
mr_id: i64,
|
||||||
|
note_id: i64,
|
||||||
|
author: &str,
|
||||||
|
body: &str,
|
||||||
|
position_new_path: Option<&str>,
|
||||||
|
) {
|
||||||
|
conn.execute(
|
||||||
|
"INSERT INTO discussions (id, gitlab_discussion_id, project_id, merge_request_id, \
|
||||||
|
noteable_type, last_seen_at)
|
||||||
|
VALUES (?1, 'disc-' || ?1, 1, ?2, 'MergeRequest', 2000)",
|
||||||
|
rusqlite::params![discussion_id, mr_id],
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
conn.execute(
|
||||||
|
"INSERT INTO notes (id, gitlab_id, discussion_id, project_id, author_username, body, \
|
||||||
|
is_system, created_at, updated_at, last_seen_at, position_new_path)
|
||||||
|
VALUES (?1, ?2, ?3, 1, ?4, ?5, 0, 1500, 1500, 2000, ?6)",
|
||||||
|
rusqlite::params![
|
||||||
|
note_id,
|
||||||
|
500 + note_id,
|
||||||
|
discussion_id,
|
||||||
|
author,
|
||||||
|
body,
|
||||||
|
position_new_path
|
||||||
|
],
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_trace_empty_file() {
|
||||||
|
let conn = setup_test_db();
|
||||||
|
seed_project(&conn);
|
||||||
|
|
||||||
|
let result = run_trace(&conn, Some(1), "src/nonexistent.rs", false, false, 10).unwrap();
|
||||||
|
assert!(result.trace_chains.is_empty());
|
||||||
|
assert_eq!(result.resolved_paths, ["src/nonexistent.rs"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_trace_finds_mr() {
|
||||||
|
let conn = setup_test_db();
|
||||||
|
seed_project(&conn);
|
||||||
|
insert_mr(&conn, 1, 10, "Add auth module", "merged", Some(3000));
|
||||||
|
insert_file_change(&conn, 1, None, "src/auth.rs", "added");
|
||||||
|
|
||||||
|
let result = run_trace(&conn, Some(1), "src/auth.rs", false, false, 10).unwrap();
|
||||||
|
assert_eq!(result.trace_chains.len(), 1);
|
||||||
|
|
||||||
|
let chain = &result.trace_chains[0];
|
||||||
|
assert_eq!(chain.mr_iid, 10);
|
||||||
|
assert_eq!(chain.mr_title, "Add auth module");
|
||||||
|
assert_eq!(chain.mr_state, "merged");
|
||||||
|
assert_eq!(chain.change_type, "added");
|
||||||
|
assert!(chain.merged_at_iso.is_some());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_trace_follows_renames() {
|
||||||
|
let conn = setup_test_db();
|
||||||
|
seed_project(&conn);
|
||||||
|
|
||||||
|
// MR 1: added old_auth.rs
|
||||||
|
insert_mr(&conn, 1, 10, "Add old auth", "merged", Some(1000));
|
||||||
|
insert_file_change(&conn, 1, None, "src/old_auth.rs", "added");
|
||||||
|
|
||||||
|
// MR 2: renamed old_auth.rs -> auth.rs
|
||||||
|
insert_mr(&conn, 2, 11, "Rename auth", "merged", Some(2000));
|
||||||
|
insert_file_change(&conn, 2, Some("src/old_auth.rs"), "src/auth.rs", "renamed");
|
||||||
|
|
||||||
|
// Query auth.rs with follow_renames -- should find both MRs
|
||||||
|
let result = run_trace(&conn, Some(1), "src/auth.rs", true, false, 10).unwrap();
|
||||||
|
|
||||||
|
assert!(result.renames_followed);
|
||||||
|
assert!(
|
||||||
|
result
|
||||||
|
.resolved_paths
|
||||||
|
.contains(&"src/old_auth.rs".to_string())
|
||||||
|
);
|
||||||
|
assert!(result.resolved_paths.contains(&"src/auth.rs".to_string()));
|
||||||
|
// MR 2 touches auth.rs (new_path), MR 1 touches old_auth.rs (new_path in its row)
|
||||||
|
assert_eq!(result.trace_chains.len(), 2);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_trace_links_issues() {
|
||||||
|
let conn = setup_test_db();
|
||||||
|
seed_project(&conn);
|
||||||
|
insert_mr(&conn, 1, 10, "Fix login bug", "merged", Some(3000));
|
||||||
|
insert_file_change(&conn, 1, None, "src/login.rs", "modified");
|
||||||
|
insert_issue(&conn, 1, 42, "Login broken on mobile", "closed");
|
||||||
|
insert_entity_ref(&conn, "merge_request", 1, "issue", 1, "closes");
|
||||||
|
|
||||||
|
let result = run_trace(&conn, Some(1), "src/login.rs", false, false, 10).unwrap();
|
||||||
|
assert_eq!(result.trace_chains.len(), 1);
|
||||||
|
assert_eq!(result.trace_chains[0].issues.len(), 1);
|
||||||
|
|
||||||
|
let issue = &result.trace_chains[0].issues[0];
|
||||||
|
assert_eq!(issue.iid, 42);
|
||||||
|
assert_eq!(issue.title, "Login broken on mobile");
|
||||||
|
assert_eq!(issue.reference_type, "closes");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_trace_limits_chains() {
|
||||||
|
let conn = setup_test_db();
|
||||||
|
seed_project(&conn);
|
||||||
|
|
||||||
|
for i in 1..=3 {
|
||||||
|
insert_mr(
|
||||||
|
&conn,
|
||||||
|
i,
|
||||||
|
10 + i,
|
||||||
|
&format!("MR {i}"),
|
||||||
|
"merged",
|
||||||
|
Some(1000 * i),
|
||||||
|
);
|
||||||
|
insert_file_change(&conn, i, None, "src/shared.rs", "modified");
|
||||||
|
}
|
||||||
|
|
||||||
|
let result = run_trace(&conn, Some(1), "src/shared.rs", false, false, 1).unwrap();
|
||||||
|
assert_eq!(result.trace_chains.len(), 1);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_trace_no_follow_renames() {
|
||||||
|
let conn = setup_test_db();
|
||||||
|
seed_project(&conn);
|
||||||
|
|
||||||
|
// MR 1: added old_name.rs
|
||||||
|
insert_mr(&conn, 1, 10, "Add old file", "merged", Some(1000));
|
||||||
|
insert_file_change(&conn, 1, None, "src/old_name.rs", "added");
|
||||||
|
|
||||||
|
// MR 2: renamed old_name.rs -> new_name.rs
|
||||||
|
insert_mr(&conn, 2, 11, "Rename file", "merged", Some(2000));
|
||||||
|
insert_file_change(
|
||||||
|
&conn,
|
||||||
|
2,
|
||||||
|
Some("src/old_name.rs"),
|
||||||
|
"src/new_name.rs",
|
||||||
|
"renamed",
|
||||||
|
);
|
||||||
|
|
||||||
|
// Without follow_renames -- should only find MR 2 (new_path = new_name.rs)
|
||||||
|
let result = run_trace(&conn, Some(1), "src/new_name.rs", false, false, 10).unwrap();
|
||||||
|
assert_eq!(result.resolved_paths, ["src/new_name.rs"]);
|
||||||
|
assert!(!result.renames_followed);
|
||||||
|
assert_eq!(result.trace_chains.len(), 1);
|
||||||
|
assert_eq!(result.trace_chains[0].mr_iid, 11);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_trace_includes_discussions() {
|
||||||
|
let conn = setup_test_db();
|
||||||
|
seed_project(&conn);
|
||||||
|
insert_mr(&conn, 1, 10, "Refactor auth", "merged", Some(3000));
|
||||||
|
insert_file_change(&conn, 1, None, "src/auth.rs", "modified");
|
||||||
|
insert_discussion_and_note(
|
||||||
|
&conn,
|
||||||
|
1,
|
||||||
|
1,
|
||||||
|
1,
|
||||||
|
"reviewer",
|
||||||
|
"This function should handle the error case.",
|
||||||
|
Some("src/auth.rs"),
|
||||||
|
);
|
||||||
|
|
||||||
|
let result = run_trace(&conn, Some(1), "src/auth.rs", false, true, 10).unwrap();
|
||||||
|
assert_eq!(result.trace_chains.len(), 1);
|
||||||
|
assert_eq!(result.trace_chains[0].discussions.len(), 1);
|
||||||
|
|
||||||
|
let disc = &result.trace_chains[0].discussions[0];
|
||||||
|
assert_eq!(disc.author_username, "reviewer");
|
||||||
|
assert!(disc.body_snippet.contains("error case"));
|
||||||
|
assert_eq!(disc.mr_iid, 10);
|
||||||
|
}
|
||||||
78
src/main.rs
78
src/main.rs
@@ -11,26 +11,26 @@ use lore::cli::autocorrect::{self, CorrectionResult};
|
|||||||
use lore::cli::commands::{
|
use lore::cli::commands::{
|
||||||
IngestDisplay, InitInputs, InitOptions, InitResult, ListFilters, MrListFilters,
|
IngestDisplay, InitInputs, InitOptions, InitResult, ListFilters, MrListFilters,
|
||||||
NoteListFilters, SearchCliFilters, SyncOptions, TimelineParams, open_issue_in_browser,
|
NoteListFilters, SearchCliFilters, SyncOptions, TimelineParams, open_issue_in_browser,
|
||||||
open_mr_in_browser, print_count, print_count_json, print_doctor_results, print_drift_human,
|
open_mr_in_browser, parse_trace_path, print_count, print_count_json, print_doctor_results,
|
||||||
print_drift_json, print_dry_run_preview, print_dry_run_preview_json, print_embed,
|
print_drift_human, print_drift_json, print_dry_run_preview, print_dry_run_preview_json,
|
||||||
print_embed_json, print_event_count, print_event_count_json, print_file_history,
|
print_embed, print_embed_json, print_event_count, print_event_count_json, print_file_history,
|
||||||
print_file_history_json, print_generate_docs, print_generate_docs_json, print_ingest_summary,
|
print_file_history_json, print_generate_docs, print_generate_docs_json, print_ingest_summary,
|
||||||
print_ingest_summary_json, print_list_issues, print_list_issues_json, print_list_mrs,
|
print_ingest_summary_json, print_list_issues, print_list_issues_json, print_list_mrs,
|
||||||
print_list_mrs_json, print_list_notes, print_list_notes_csv, print_list_notes_json,
|
print_list_mrs_json, print_list_notes, print_list_notes_csv, print_list_notes_json,
|
||||||
print_list_notes_jsonl, print_search_results, print_search_results_json, print_show_issue,
|
print_list_notes_jsonl, print_search_results, print_search_results_json, print_show_issue,
|
||||||
print_show_issue_json, print_show_mr, print_show_mr_json, print_stats, print_stats_json,
|
print_show_issue_json, print_show_mr, print_show_mr_json, print_stats, print_stats_json,
|
||||||
print_sync, print_sync_json, print_sync_status, print_sync_status_json, print_timeline,
|
print_sync, print_sync_json, print_sync_status, print_sync_status_json, print_timeline,
|
||||||
print_timeline_json_with_meta, print_who_human, print_who_json, query_notes, run_auth_test,
|
print_timeline_json_with_meta, print_trace, print_trace_json, print_who_human, print_who_json,
|
||||||
run_count, run_count_events, run_doctor, run_drift, run_embed, run_file_history,
|
query_notes, run_auth_test, run_count, run_count_events, run_doctor, run_drift, run_embed,
|
||||||
run_generate_docs, run_ingest, run_ingest_dry_run, run_init, run_list_issues, run_list_mrs,
|
run_file_history, run_generate_docs, run_ingest, run_ingest_dry_run, run_init, run_list_issues,
|
||||||
run_search, run_show_issue, run_show_mr, run_stats, run_sync, run_sync_status, run_timeline,
|
run_list_mrs, run_search, run_show_issue, run_show_mr, run_stats, run_sync, run_sync_status,
|
||||||
run_who,
|
run_timeline, run_who,
|
||||||
};
|
};
|
||||||
use lore::cli::render::{ColorMode, GlyphMode, Icons, LoreRenderer, Theme};
|
use lore::cli::render::{ColorMode, GlyphMode, Icons, LoreRenderer, Theme};
|
||||||
use lore::cli::robot::{RobotMeta, strip_schemas};
|
use lore::cli::robot::{RobotMeta, strip_schemas};
|
||||||
use lore::cli::{
|
use lore::cli::{
|
||||||
Cli, Commands, CountArgs, EmbedArgs, FileHistoryArgs, GenerateDocsArgs, IngestArgs, IssuesArgs,
|
Cli, Commands, CountArgs, EmbedArgs, FileHistoryArgs, GenerateDocsArgs, IngestArgs, IssuesArgs,
|
||||||
MrsArgs, NotesArgs, SearchArgs, StatsArgs, SyncArgs, TimelineArgs, WhoArgs,
|
MrsArgs, NotesArgs, SearchArgs, StatsArgs, SyncArgs, TimelineArgs, TraceArgs, WhoArgs,
|
||||||
};
|
};
|
||||||
use lore::core::db::{
|
use lore::core::db::{
|
||||||
LATEST_SCHEMA_VERSION, create_connection, get_schema_version, run_migrations,
|
LATEST_SCHEMA_VERSION, create_connection, get_schema_version, run_migrations,
|
||||||
@@ -40,8 +40,10 @@ use lore::core::error::{LoreError, RobotErrorOutput};
|
|||||||
use lore::core::logging;
|
use lore::core::logging;
|
||||||
use lore::core::metrics::MetricsLayer;
|
use lore::core::metrics::MetricsLayer;
|
||||||
use lore::core::paths::{get_config_path, get_db_path, get_log_dir};
|
use lore::core::paths::{get_config_path, get_db_path, get_log_dir};
|
||||||
|
use lore::core::project::resolve_project;
|
||||||
use lore::core::shutdown::ShutdownSignal;
|
use lore::core::shutdown::ShutdownSignal;
|
||||||
use lore::core::sync_run::SyncRunRecorder;
|
use lore::core::sync_run::SyncRunRecorder;
|
||||||
|
use lore::core::trace::run_trace;
|
||||||
|
|
||||||
#[tokio::main]
|
#[tokio::main]
|
||||||
async fn main() {
|
async fn main() {
|
||||||
@@ -199,6 +201,7 @@ async fn main() {
|
|||||||
Some(Commands::FileHistory(args)) => {
|
Some(Commands::FileHistory(args)) => {
|
||||||
handle_file_history(cli.config.as_deref(), args, robot_mode)
|
handle_file_history(cli.config.as_deref(), args, robot_mode)
|
||||||
}
|
}
|
||||||
|
Some(Commands::Trace(args)) => handle_trace(cli.config.as_deref(), args, robot_mode),
|
||||||
Some(Commands::Drift {
|
Some(Commands::Drift {
|
||||||
entity_type,
|
entity_type,
|
||||||
iid,
|
iid,
|
||||||
@@ -725,6 +728,7 @@ fn suggest_similar_command(invalid: &str) -> String {
|
|||||||
("note", "notes"),
|
("note", "notes"),
|
||||||
("drift", "drift"),
|
("drift", "drift"),
|
||||||
("file-history", "file-history"),
|
("file-history", "file-history"),
|
||||||
|
("trace", "trace"),
|
||||||
];
|
];
|
||||||
|
|
||||||
let invalid_lower = invalid.to_lowercase();
|
let invalid_lower = invalid.to_lowercase();
|
||||||
@@ -766,6 +770,7 @@ fn command_example(cmd: &str) -> &'static str {
|
|||||||
"generate-docs" => "lore --robot generate-docs",
|
"generate-docs" => "lore --robot generate-docs",
|
||||||
"embed" => "lore --robot embed",
|
"embed" => "lore --robot embed",
|
||||||
"robot-docs" => "lore robot-docs",
|
"robot-docs" => "lore robot-docs",
|
||||||
|
"trace" => "lore --robot trace src/main.rs",
|
||||||
"init" => "lore init",
|
"init" => "lore init",
|
||||||
_ => "lore --robot <command>",
|
_ => "lore --robot <command>",
|
||||||
}
|
}
|
||||||
@@ -1888,6 +1893,51 @@ fn handle_file_history(
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn handle_trace(
|
||||||
|
config_override: Option<&str>,
|
||||||
|
args: TraceArgs,
|
||||||
|
robot_mode: bool,
|
||||||
|
) -> Result<(), Box<dyn std::error::Error>> {
|
||||||
|
let start = std::time::Instant::now();
|
||||||
|
let config = Config::load(config_override)?;
|
||||||
|
|
||||||
|
let (path, line_requested) = parse_trace_path(&args.path);
|
||||||
|
|
||||||
|
if line_requested.is_some() && !robot_mode {
|
||||||
|
eprintln!(
|
||||||
|
"Note: Line-level tracing requires Tier 2 (git blame). Showing file-level results."
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
let project = config
|
||||||
|
.effective_project(args.project.as_deref())
|
||||||
|
.map(String::from);
|
||||||
|
|
||||||
|
let db_path = get_db_path(config.storage.db_path.as_deref());
|
||||||
|
let conn = create_connection(&db_path)?;
|
||||||
|
let project_id = project
|
||||||
|
.as_deref()
|
||||||
|
.map(|p| resolve_project(&conn, p))
|
||||||
|
.transpose()?;
|
||||||
|
|
||||||
|
let result = run_trace(
|
||||||
|
&conn,
|
||||||
|
project_id,
|
||||||
|
&path,
|
||||||
|
!args.no_follow_renames,
|
||||||
|
args.discussions,
|
||||||
|
args.limit,
|
||||||
|
)?;
|
||||||
|
|
||||||
|
if robot_mode {
|
||||||
|
let elapsed_ms = start.elapsed().as_millis() as u64;
|
||||||
|
print_trace_json(&result, elapsed_ms, line_requested);
|
||||||
|
} else {
|
||||||
|
print_trace(&result);
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
async fn handle_timeline(
|
async fn handle_timeline(
|
||||||
config_override: Option<&str>,
|
config_override: Option<&str>,
|
||||||
args: TimelineArgs,
|
args: TimelineArgs,
|
||||||
@@ -2556,6 +2606,16 @@ fn handle_robot_docs(robot_mode: bool, brief: bool) -> Result<(), Box<dyn std::e
|
|||||||
"active_minimal": ["entity_type", "iid", "title", "participants"]
|
"active_minimal": ["entity_type", "iid", "title", "participants"]
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
"trace": {
|
||||||
|
"description": "Trace why code was introduced: file -> MR -> issue -> discussion. Follows rename chains by default.",
|
||||||
|
"flags": ["<path>", "-p/--project <path>", "--discussions", "--no-follow-renames", "-n/--limit <N>"],
|
||||||
|
"example": "lore --robot trace src/main.rs -p group/repo",
|
||||||
|
"response_schema": {
|
||||||
|
"ok": "bool",
|
||||||
|
"data": {"path": "string", "resolved_paths": "[string]", "trace_chains": "[{mr_iid:int, mr_title:string, mr_state:string, mr_author:string, change_type:string, merged_at_iso:string?, updated_at_iso:string, web_url:string?, issues:[{iid:int, title:string, state:string, reference_type:string, web_url:string?}], discussions:[{discussion_id:string, mr_iid:int, author_username:string, body_snippet:string, path:string, created_at_iso:string}]}]"},
|
||||||
|
"meta": {"tier": "string (api_only)", "line_requested": "int?", "elapsed_ms": "int", "total_chains": "int", "renames_followed": "bool"}
|
||||||
|
}
|
||||||
|
},
|
||||||
"file-history": {
|
"file-history": {
|
||||||
"description": "Show MRs that touched a file, with rename chain resolution and optional DiffNote discussions",
|
"description": "Show MRs that touched a file, with rename chain resolution and optional DiffNote discussions",
|
||||||
"flags": ["<path>", "-p/--project <path>", "--discussions", "--no-follow-renames", "--merged", "-n/--limit <N>"],
|
"flags": ["<path>", "-p/--project <path>", "--discussions", "--no-follow-renames", "--merged", "-n/--limit <N>"],
|
||||||
|
|||||||
Reference in New Issue
Block a user