Skip to content

docs: ROADMAP from issues + gh CLI workflow; parsing diff-prefix repair (#28) #43

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
haasonsaas merged 1 commit into from
Mar 14, 2026
Merged

docs: ROADMAP from issues + gh CLI workflow; parsing diff-prefix repair (#28) #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ cargo run -- --help # CLI usage
- `migrations/` — PostgreSQL migrations (sqlx)
- `eval/` — Evaluation and benchmarking
- `examples/` — Usage examples
- `docs/` — Mutation testing, **ROADMAP.md** (enhancement backlog + gh CLI workflow)

## Development
- Run `bash scripts/install-hooks.sh` once to install pre-commit and pre-push hooks (merge conflict check, trailing newline, conditional Rust/web checks, shellcheck; pre-push adds version sync, cargo audit, full web build+test). Mutation testing runs in CI only; see `docs/mutation-testing.md`.
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,7 @@ The summary includes:

## Contributing

Contributions are welcome! Please open an issue first to discuss what you would like to change.
Contributions are welcome! Please open an issue first to discuss what you would like to change. Enhancement backlog and triage: see [docs/ROADMAP.md](docs/ROADMAP.md) and `gh issue list --label "priority: high"`.

**PR workflow:** Open a PR → ensure CI is green (version, lint, security, test, mutation, review) → merge when ready. Use a short test plan in the PR description. Small, focused PRs are preferred.

Expand Down
85 changes: 85 additions & 0 deletions docs/ROADMAP.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# DiffScope roadmap

Enhancement backlog derived from open GitHub issues. Use **gh CLI** for triage and filtering.

## gh CLI workflow

```bash
# List open issues (default: 30)
gh issue list

# Filter by label
gh issue list --label "priority: high"
gh issue list --label "area: review-pipeline"

# Search (advanced filters)
gh issue list --search "no:assignee sort:created-asc"
gh issue list --search "verification OR RAG"

# View one issue
gh issue view 32

# Add/remove labels (labels must exist: gh label create "name" --color "hex" --description "desc")
gh issue edit 32 --add-label "priority: medium,area: plugins"
gh issue edit 32 --remove-label "help wanted"

# Add to project (requires project scope)
gh issue edit 32 --add-project "Roadmap"
```

Create labels once: `priority: high`, `priority: medium`, `priority: low`, `area: review-pipeline`, `area: plugins`, `area: platform`.

---

## Priority: High / Critical

| # | Title | Area | Notes |
|---|--------|------|--------|
| [27](https://github.com/evalops/diffscope/issues/27) | Embedding-based false positive filtering from developer feedback | review | Greptile-style: block if similar to 3+ downvoted; pass if 3+ upvoted. Per-team. |
| [23](https://github.com/evalops/diffscope/issues/23) | Verification pass to catch hallucinations | review | Second LLM pass validates findings vs actual code; drop below score. **Partially done** (verification in config/pipeline). |
| [22](https://github.com/evalops/diffscope/issues/22) | Embedding-based RAG pipeline with function-level chunking | review | NL summaries + pgvector; highest leverage for catch rate. |
| [24](https://github.com/evalops/diffscope/issues/24) | Agentic review loop with tool use | review | Tools: search_code, read_file, search_symbols, git_log, git_blame. |
| [21](https://github.com/evalops/diffscope/issues/21) | Multi-agent architecture: review + fix + test agents | platform | Fix Agent, Test Agent, Triage Agent; orchestration. |
| [10](https://github.com/evalops/diffscope/issues/10) | Deep codebase graph context in review prompts | review | Pre-index repo; inject callers/callees/contracts into prompt. |

## Priority: Medium

| # | Title | Area | Notes |
|---|--------|------|--------|
| [32](https://github.com/evalops/diffscope/issues/32) | In-sandbox linter/analyzer execution | plugins | ToolSandbox + AnalysisTool trait; Clippy, Ruff, Gitleaks, ShellCheck, actionlint. |
| [31](https://github.com/evalops/diffscope/issues/31) | AST-based structural pattern matching (ast-grep) | plugins | Pre-analyzer plugin; coderabbitai/ast-grep-essentials rules. |
| [30](https://github.com/evalops/diffscope/issues/30) | Adaptive patch compression for large PRs | review | Full → Compressed → Clipped → MultiCall; token budget. |
| [29](https://github.com/evalops/diffscope/issues/29) | File triage: classify before expensive review | review | NeedsReview vs Cosmetic/ConfigChange/TestOnly; heuristic + cheap model. |
| [28](https://github.com/evalops/diffscope/issues/28) | Robust LLM output parsing with fallback strategies | review | **In progress:** code-block extraction, trailing commas, diff-prefix strip. More fallbacks in parsing/llm_response.rs. |
| [25](https://github.com/evalops/diffscope/issues/25) | Dynamic context: enclosing function/class boundary | review | Search upward for boundary; reuse symbol_index patterns. |

## Priority: Low / Tier 2–3

| # | Title | Area | Notes |
|---|--------|------|--------|
| [20](https://github.com/evalops/diffscope/issues/20) | Built-in secrets detection scanner | plugins | **Done:** `secret_scanner.rs` with AWS, GitHub, Slack, JWT, PEM, etc. |
| [19](https://github.com/evalops/diffscope/issues/19) | Compliance review command | platform | `diffscope compliance` — security, secrets, rules, ticket, licenses, duplication. |
| [18](https://github.com/evalops/diffscope/issues/18) | Authentication layer for web UI (SSO/SAML) | platform | Basic → OAuth/OIDC → SAML, RBAC. |
| [17](https://github.com/evalops/diffscope/issues/17) | GitLab, Azure DevOps, Bitbucket support | platform | GitPlatform trait; GitLab first. |
| [15](https://github.com/evalops/diffscope/issues/15) | Auto-generate Mermaid sequence diagrams in PRs | review | Symbol graph → sequence diagram in PR comment. |
| [14](https://github.com/evalops/diffscope/issues/14) | VS Code / IDE extension | platform | Staged/unstaged review, inline diagnostics, Quick Fix. |
| [13](https://github.com/evalops/diffscope/issues/13) | Ticket validation (Jira/Linear/GitHub Issues) | platform | Fetch ticket, validate acceptance criteria. |
| [12](https://github.com/evalops/diffscope/issues/12) | Natural language custom review rules | review | Prose rules in YAML or `.diffscope-rules`. |
| [11](https://github.com/evalops/diffscope/issues/11) | PR analytics and review metrics dashboard | platform | Persist to PG, aggregation, dashboard. |
| [9](https://github.com/evalops/diffscope/issues/9) | Structured PR description auto-generation | platform | `pr describe` — walkthrough, labels, breaking, testing notes. |

---

## Shipped (recent)

- **LLM parsing (#28):** Repair candidate for diff-style line prefixes (`+` on each line) in `repair_json_candidates`; test `parse_json_with_diff_prefix_artifact`.
- **Secrets (#20):** Built-in secret scanner in `plugins/builtin/secret_scanner.rs`.
- **Verification (#23):** Verification pass and config (verification.*) in pipeline.

---

## References

- [GitHub CLI manual](https://cli.github.com/manual/)
- [Advanced issue search](https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests)
- [gh issue edit](https://cli.github.com/manual/gh_issue_edit) — labels, assignees, projects, milestones
35 changes: 30 additions & 5 deletions src/parsing/llm_response.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,13 +347,27 @@ fn find_balanced_json(content: &str, open: char, close: char) -> Option<String>
fn repair_json_candidates(candidate: &str) -> Vec<String> {
static TRAILING_COMMAS: Lazy<Regex> = Lazy::new(|| Regex::new(r",\s*([}\]])").unwrap());

let mut candidates = vec![candidate.trim().to_string()];
let without_trailing_commas = TRAILING_COMMAS
.replace_all(candidate.trim(), "$1")
.to_string();
if without_trailing_commas != candidate.trim() {
let trimmed = candidate.trim();
let mut candidates = vec![trimmed.to_string()];

let without_trailing_commas = TRAILING_COMMAS.replace_all(trimmed, "$1").to_string();
if without_trailing_commas != trimmed {
candidates.push(without_trailing_commas);
}

// When LLM echoes JSON with diff-style line prefixes (leading "+"), strip them (issue #28).
let without_diff_prefix: String = trimmed
.lines()
.map(|line| line.strip_prefix('+').map(str::trim).unwrap_or(line))
.collect::<Vec<_>>()
.join("\n");
let without_diff_prefix = without_diff_prefix.trim();
if without_diff_prefix != trimmed
&& (without_diff_prefix.starts_with('[') || without_diff_prefix.starts_with('{'))
{
candidates.push(without_diff_prefix.to_string());
}
Comment on lines +359 to +369
Copy link

@sentry sentry bot Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The logic in repair_json_candidates to strip + prefixes from JSON is unreachable because upstream functions filter out or bypass the +-prefixed content.
Severity: MEDIUM

Suggested Fix

The fix should be applied in extract_json_from_code_block. The condition that checks if the content starts with [ or { should be modified to also account for content where lines might have + prefixes before the JSON structure begins.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/parsing/llm_response.rs#L358-L369

Potential issue: The new code in `repair_json_candidates` intended to strip diff-style
`+` prefixes from JSON content is unreachable. The upstream function
`extract_json_from_code_block` filters out any content that does not start with `[` or
`{`, so `+`-prefixed strings are never passed down. Fallback functions like
`find_json_array` extract valid JSON from within the prefixed lines, bypassing the new
repair logic entirely. As a result, the feature does not work as intended, and the
associated test passes due to a fallback mechanism, not because the new code is
functioning correctly.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link

@cursor cursor bot Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diff-prefix repair unreachable for multi-line JSON input

Medium Severity

The diff-prefix stripping in repair_json_candidates is unreachable for its intended multi-line use case. extract_json_from_code_block rejects code block content starting with + (the starts_with('[') || starts_with('{') guard at line 288), and find_balanced_json can't return content containing embedded + characters because serde validation fails. The repair lives downstream of extraction, so content that needs repairing never reaches it. The test parse_json_with_diff_prefix_artifact passes only because find_json_array locates the valid […] substring directly, never exercising the new repair code.

Additional Locations (2)
Fix in Cursor Fix in Web

Copy link

@cursor cursor bot Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Repair candidates not composed, combined fixes missed

Low Severity

The diff-prefix stripping candidate is derived from trimmed (original input), and the trailing-comma candidate is also derived from trimmed. Neither repair is applied to the other's output, so if the extracted JSON needs both fixes (e.g., +-prefixed lines and trailing commas), no single candidate in the returned Vec will be valid JSON and parsing silently fails. The diff-prefix candidate also needs trailing-comma removal applied to it.

Fix in Cursor Fix in Web


candidates
}

Expand Down Expand Up @@ -1218,6 +1232,17 @@ let data = &input;
assert_eq!(comments[0].line_number, 5);
}

#[test]
fn parse_json_with_diff_prefix_artifact() {
// LLM sometimes echoes JSON in a code block with leading "+" on each line (diff artifact); repair strips (issue #28).
let input = "```json\n+[{\"line\": 7, \"issue\": \"Missing check\"}]\n+\n```";
let file_path = PathBuf::from("src/lib.rs");
let comments = parse_llm_response(input, &file_path).unwrap();
assert_eq!(comments.len(), 1);
assert_eq!(comments[0].line_number, 7);
assert!(comments[0].content.contains("Missing check"));
}

// ── Bug: find_json_array uses mismatched brackets ──────────────────
//
// `find_json_array` uses `find('[')` (first) + `rfind(']')` (last).
Expand Down
Loading