Commit graph

5543 commits

Author SHA1 Message Date
wenshao
608550f2fc fix(review): restore strict parallel agent launch instruction
The relaxed "if you cannot fit 5, try 3+2" gave the model a fallback
that it always took (serial execution). Restored strict requirement:
MUST include all 5 tool calls in one response. The runtime confirms
concurrent agent execution is supported.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 09:19:03 +08:00
wenshao
205eefe6f9 Merge remote-tracking branch 'origin/main' into feat/review-skill-improvements 2026-04-07 09:15:18 +08:00
Shaojin Wen
b6373ac71e
feat(core): implement mid-turn queue drain for agent execution (#2854)
Some checks are pending
Qwen Code CI / CodeQL (push) Waiting to run
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (push) Blocked by required conditions
Qwen Code CI / Test-1 (push) Blocked by required conditions
Qwen Code CI / Test-2 (push) Blocked by required conditions
Qwen Code CI / Test-3 (push) Blocked by required conditions
Qwen Code CI / Test-4 (push) Blocked by required conditions
Qwen Code CI / Test-5 (push) Blocked by required conditions
Qwen Code CI / Test-6 (push) Blocked by required conditions
Qwen Code CI / Test-7 (push) Blocked by required conditions
Qwen Code CI / Test-8 (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(core): implement mid-turn queue drain for agent execution

Inject queued user messages between tool execution steps within a single
turn, so the model sees them immediately instead of waiting for the
entire round to complete.

- Add `dequeueAll()` to AsyncMessageQueue
- Add `midTurnDrain` callback to ReasoningLoopOptions
- Drain queue after processFunctionCalls, inject as text parts
- AgentComposer always enqueues directly (no local buffering)
- Add QUEUE_MESSAGES_CONSUMED event for UI sync

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(cli): add mid-turn queue drain to main session

Extend mid-turn queue drain to the main session's tool execution path
(useGeminiStream). Previously only agent tabs had this feature.

- Add midTurnDrainRef parameter to useGeminiStream
- Inject queued messages in handleCompletedTools before submitQuery
- Bridge useMessageQueue to drain ref in AppContainer via ref pattern

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address Copilot review feedback on mid-turn drain

- Guard midTurnDrain with abort check to prevent message loss on cancel
- Synchronously clear messageQueueRef to prevent duplicate drains
- Only clear pending display on IDLE status, not all status changes

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: scope mid-turn drain to main session only

Revert subagent-path changes (AgentCore, AgentInteractive,
AgentComposer, AsyncMessageQueue, agent-events) to keep the PR
focused on the main session, which is easier to test and validate.

Subagent mid-turn drain can be added in a follow-up PR.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address Copilot review on main session mid-turn drain

- Move synchronous queue ref into useMessageQueue itself, expose
  drainQueue() for atomic drain (fixes race between addMessage and drain)
- Record drained messages as USER history items so the transcript
  stays complete
- Simplify AppContainer bridge to just midTurnDrainRef.current = drainQueue

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: guard mid-turn drain against cancelled turns

- Skip drain when turnCancelledRef or abortController signal is set,
  so queued messages stay for the next turn instead of being lost
- Restore ref-based queue bridge (drainQueue removed from useMessageQueue)
- Keep synchronous ref clear to prevent duplicate drains

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 09:14:44 +08:00
wenshao
446d309d53 fix(review): pragmatic parallel agent launch instruction
The strict "all 5 in one response" requirement may exceed the model's
output token limit. Changed to: prefer all 5 in one response, but
allow splitting (e.g., 3+2) as long as agents launch without waiting
for previous ones to finish. Runtime confirms parallel execution is
supported (coreToolScheduler tests).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 09:14:12 +08:00
wenshao
e96de40754 fix(review): handle partial inline comment failures + accept *italic*
1. When verdict is Comment and SOME inline comments fail, submit
   a summary with the failed findings (not lost). Only skip summary
   when ALL inline comments succeed.
2. Accept *italic* for model attribution in prose — prettier
   normalizes _italic_ to *italic* in markdown, both render the same.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 09:06:30 +08:00
wenshao
88ff2ac034 fix(review): prefer Maven/Gradle wrappers (./mvnw, ./gradlew)
Many repos rely on wrappers and don't require global Maven/Gradle
installed. Now prefers ./mvnw over mvn and ./gradlew over gradle
in both Step 3 (linter) and Agent 5 (build/test).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 09:00:07 +08:00
wenshao
b9d3602309 fix(review): 3 Copilot comments — conditional cleanup, italic format, cache SHA
1. Step 11: conditional worktree removal — skip if Step 8 flagged
   preservation (autofix commit/push failure)
2. Standardize model attribution to _italic_ (was mixed *italic*)
3. Cache stores pre-autofix headRefOid (not worktree HEAD which may
   include the autofix commit)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 08:58:13 +08:00
wenshao
226c9e2c19 fix(review): CI config discovery runs for ALL projects, not just unmatched
Previously, rule 7 (CI config auto-discovery) only ran for "unrecognized
projects." Java/Makefile (e.g., OpenJDK) and C/C++/Makefile matched
earlier rules that said "skip," so CI config was never read.

Now: language-specific rules 1-6 run first for known tools, then rule 7
runs for ALL projects to discover additional CI-defined checks. OpenJDK
will now discover jcheck and custom targets from .github/workflows/*.yml.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 08:53:46 +08:00
wenshao
44241bc8c1 fix(review): 3 issues from self-audit
1. Step 9 code block: annotate Comment template applies only when
   no inline comments were posted (avoids LLM confusion)
2. Step 11: fix stale rationale — Step 9 uses API calls, not worktree
3. Verdict: explicitly based on high-confidence findings only —
   low-confidence findings don't influence PR approval status

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 07:56:52 +08:00
wenshao
57b7353946 fix(review): skip redundant review summary when inline comments suffice
When verdict is "Comment" and inline comments were posted successfully,
do NOT submit an additional gh pr review — the inline comments are
sufficient. Only submit summary for Approve/Request changes (which
carry approval status) or when no inline comments were posted.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 07:53:38 +08:00
wenshao
dcb58545d2 feat(review): model attribution on inline comments + minimal summary
1. Each inline PR comment now includes model attribution footer
   (e.g., "— qwen3-coder") so reviewers know which model produced
   each comment.
2. When inline comments are posted successfully, the review summary
   is minimal (verdict + model only, no repeated findings). Full
   summary is only used when no inline comments were posted.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 07:52:40 +08:00
wenshao
8f723575bd docs: add CI config auto-discovery to user doc and design doc
- User doc: added "Other" row to language table + explanation that
  CI config is read for unrecognized projects
- DESIGN.md: added "Why auto-discover from CI config" decision
  section + added .qwen/review-tools.md to rejected alternatives

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 07:47:07 +08:00
wenshao
2595567585 feat(review): auto-discover lint/build/test from CI config
For projects that don't match standard tool patterns (e.g., OpenJDK),
Step 3 and Agent 5 now read CI configuration files (.github/workflows,
.gitlab-ci.yml, Jenkinsfile, Makefile) to discover what lint/build/test
commands the project uses. No user configuration needed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 07:44:48 +08:00
wenshao
cb359dc996 fix(review): default remote for PR numbers + fork-aware autofix push
1. Pure integer PR numbers default to origin (no URL-based remote
   matching available). URL-based PRs use the matched remote.
2. Autofix push uses matched remote instead of hardcoded origin.
   Push failure is expected for fork PRs where user lacks push
   access — handled gracefully with informative message.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 07:25:48 +08:00
wenshao
f357517dd2 fix(review): temp file cleanup in cross-repo + base-branch remote
1. Cross-repo lightweight mode: no longer skips Step 11 entirely.
   Worktree removal is skipped (none created) but temp files are
   still cleaned up.
2. Step 2 base-branch fetch: use the matched remote from Step 1
   (e.g., upstream for forks) instead of hardcoded origin.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 07:23:47 +08:00
wenshao
cbf2aa06ac feat(review): add Java and C/C++ support for deterministic analysis
Step 3 now supports:
- Java: mvn compile, checkstyle, spotbugs, pmd (Maven);
  gradle compileJava, checkstyleMain (Gradle)
- C/C++: clang-tidy (when compile_commands.json available)

Agent 5 build/test precedence now includes Maven and Gradle
before Makefile to avoid duplicate builds.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 07:19:56 +08:00
wenshao
b2ec7b068e fix(review): check all git remotes for cross-repo detection
Previously compared URL owner/repo against gh repo view (current repo
only). This caused fork-based workflows to be wrongly classified as
cross-repo (e.g., local clone of wenshao/jdk reviewing openjdk/jdk PR).

Now checks all git remotes (git remote -v) for a match. If any remote
points to the URL's repo, proceeds with full worktree mode using that
remote for fetch. Only falls back to lightweight mode if NO remote
matches.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 07:14:20 +08:00
Shaojin Wen
5df8fa0ff2
Merge pull request #2889 from wenshao/feat/dangerous-actions-guidance
feat(prompt): add dangerous actions behavior guidance in system prompt
2026-04-07 06:58:50 +08:00
wenshao
3825962a8b fix(review): Step 9 owner/repo must respect cross-repo mode
Step 9 hardcoded gh repo view to get owner/repo, which returns the
current repo — wrong for cross-repo reviews. Now explicitly branches:
same-repo uses gh repo view, cross-repo uses URL-extracted owner/repo.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 05:07:32 +08:00
wenshao
720796e699 fix(review): safe branch cleanup + cross-repo agent count
- git branch -D: add 2>/dev/null || true to both cleanup sites
  (Step 1 stale cleanup + Step 11) to prevent abort if ref missing
- Cross-repo doc: clarify Agents 1-4 only (Agent 5 build/test
  requires local codebase, not available in cross-repo mode)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 05:02:13 +08:00
wenshao
6596362342 fix(review): 4 issues found in self-audit
1. Line 22: "Step 2" → "Step 4" (agents are Step 4, rules are Step 2)
2. PR section: clarify "same-repo URL" to avoid ambiguity with
   cross-repo lightweight path
3. Cross-file analysis: add "same-repo only" qualifier (no local
   files in cross-repo mode for grep_search)
4. Cross-repo lightweight mode: add existing PR comment fetching
   to avoid duplicating human feedback

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 04:24:45 +08:00
wenshao
875944544c fix(review): explicitly require parallel agent launch in single batch
Strengthen the parallel instruction from "five parallel review agents"
to explicitly requiring all 5 task tool calls in one response. Without
this, the LLM may serialize agents (wait for each to finish), losing
the wall-clock time benefit of parallel execution.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 04:22:34 +08:00
wenshao
559f2efd42 fix(review): fix cross-repo mode and add documentation
SKILL.md:
- Step 9 must use owner/repo from URL (not gh repo view) for cross-repo
- Step 2 (project rules) skipped in cross-repo mode (no local files)

User doc: add Cross-repo PR Review section with same-repo vs cross-repo
capability comparison table.

DESIGN.md: add "Why cross-repo uses lightweight mode" section explaining
CLI tools are inherently repo-local and our approach is best available.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 04:18:55 +08:00
wenshao
0e78ced15a fix(review): support cross-repo PR URLs in lightweight mode
Instead of rejecting cross-repo PR URLs, run in lightweight mode:
use gh pr diff <url> to get diff directly (no worktree needed),
skip deterministic analysis and build/test, and review diff text
only with LLM agents.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 04:16:06 +08:00
wenshao
afbede1d34 fix(review): detect and reject cross-repo PR URLs
When a PR URL points to a different repo (e.g., other-org/other-repo),
the review would silently operate on the wrong PR in the current repo.
Now verifies URL owner/repo matches current repo before proceeding.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 04:15:40 +08:00
wenshao
af4ae2cb83 fix: update stale Step 1.5 reference to Step 3 in DESIGN.md
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 04:13:28 +08:00
wenshao
01c105b5e7 refactor(review): renumber steps from sub-steps to sequential 1-11
Replace confusing sub-step numbering (1, 1.1, 1.5, 2, 2.5, 2.6, 3,
3.5, 4, 4.5, 5) with clean sequential numbering (1-11).

Mapping: 1→1, 1.1→2, 1.5→3, 2→4, 2.5→5, 2.6→6, 3→7, 3.5→8,
4→9, 4.5→10, 5→11

Updated all cross-references in SKILL.md, user docs, and PR description.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 04:12:45 +08:00
wenshao
b4ae04ec29 fix(review): capture headRefOid before autofix to prevent line drift
If autofix pushes a new commit in Step 3.5, the PR HEAD changes.
Step 4's inline comments would then reference the autofix commit
where line numbers may have shifted, causing comments on wrong lines.

Fix: capture headRefOid in Step 1 (before autofix) and reuse in
Step 4. Also fix stale Step 5 comment about worktree/commit SHA.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:57:14 +08:00
wenshao
e5127e6fc0 fix(review): shell safety for no-findings path, fix DESIGN.md table
- No-findings LGTM path now uses write_file + --body-file instead
  of inline --body (consistent with shell safety guidance)
- DESIGN.md: remove duplicate Agent 5 row from token table
- PR description: add DESIGN.md to scope list

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:54:57 +08:00
wenshao
fd3d2a8359 docs: add Fork Subagent as future optimization in DESIGN.md
Current 7 LLM calls send ~350K input tokens (50K system prompt × 7
independent subagents). Fork Subagent would share prompt cache across
all forks, reducing to ~120K effective tokens (~65% savings).

Documented as future optimization pending Fork Subagent platform
feature implementation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:48:35 +08:00
wenshao
09c95c44c8 fix(review): address 5 Copilot comments
- Add error handling for git fetch and gh pr view failures in Step 1
- Skip worktree cleanup on autofix commit/push failure (preserve
  uncommitted fixes for manual recovery)
- Fix Agent 5 counting: it's 1 of the 5 LLM agents (not a separate
  zero-cost stage). Remove misleading "zero LLM" annotation and
  duplicate row from token efficiency table.
- Reverse audit skip-verification already implemented (comment #53
  was stale)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:42:20 +08:00
wenshao
ccaab1779b docs: add /review design document with architecture rationale
Records the WHY behind key decisions:
- Why 5 agents (not 1 like Copilot): dimensional coverage
- Why batch verification (not N agents): O(1) not O(N) cost
- Why reverse audit is separate: different cognitive task
- Why worktree (not stash/checkout): eliminates a class of bugs
- Why "silence > noise": Copilot's 60M review data
- Why base-branch rules: security against PR injection
- Why follow-up tips (not blocking prompts): UX consistency
- Why 7 fixed LLM calls: coverage vs cost balance
- Rejected alternatives table with reasoning

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:40:17 +08:00
wenshao
6d7afafe3c perf(review): skip verification for reverse audit findings
Reverse audit agent already has full context (all confirmed findings +
entire diff), so its findings don't need a second opinion. This brings
the actual LLM call count to 7 (5 review + 1 verify + 1 reverse),
matching the documented claim.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:38:49 +08:00
wenshao
d6b9b35350 fix(review): handle interrupted review cleanup
If a previous review was interrupted (Ctrl+C, crash), stale worktree
and local ref would block the next review. Now Step 1 checks for and
cleans up stale .qwen/tmp/review-pr-<N> worktree and qwen-review/pr-<N>
ref before creating new ones.

Step 5 also cleans up the local ref alongside the worktree.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:35:00 +08:00
wenshao
6b28920a07 docs: fix autofix section redundancy and add pre-fix verdict note
- Remove duplicate worktree commit+push bullet (lines 107 vs 109)
- Add note that PR submission uses pre-fix verdict since remote
  isn't updated until autofix push completes

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:31:48 +08:00
wenshao
ce47f64ae6 docs: replace Mermaid with plain-text pipeline diagram
Mermaid only renders on GitHub; shows as raw code on Nextra,
Docusaurus, VS Code preview, and offline viewing. Plain-text
ASCII diagram is universally compatible and includes LLM call
cost annotations on each stage.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:30:34 +08:00
wenshao
1db1dec517 docs: replace step list with Mermaid flowchart in How It Works
Visual pipeline diagram showing:
- Sequential flow from scope detection to cleanup
- 5 parallel agents subgraph
- Decision branches for autofix and PR comments
- Zero-LLM-cost stages marked
- GitHub renders Mermaid natively

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:29:20 +08:00
wenshao
295e907d25 fix(review): address 4 Copilot comments on worktree and verification
- Step 4.5: use absolute paths for reports/cache in worktree mode
  (relative paths would land in worktree and be deleted)
- Step 1: fetch into qwen-review/pr-<N> ref to avoid clobbering
  existing local branches
- Step 2.6: reverse audit findings use batch verification (not
  one-per-finding), consistent with Step 2.5
- Doc: clarify reverse audit findings are also batch-verified

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:25:24 +08:00
wenshao
9b9bccd27d docs: update user doc with token efficiency, fix follow-up table
- Add Token Efficiency section showing fixed 7 LLM calls breakdown
- Fix follow-up table: "fix these issues" is local-only (worktree
  cleaned up after PR review)
- Update PR description with worktree, batch verification, cross-model
  review, PR comment dedup, and expanded test plan

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:22:54 +08:00
wenshao
e65e5bd353 perf(review): replace N verification agents with single batch verification
Previously, each finding got its own independent verification agent
(N findings = N LLM calls). Now a single verification agent receives
all findings at once and verifies them in one pass.

Token cost: 6+N variable calls → 7 fixed calls (5 review + 1 verify + 1 reverse audit)
Quality: minimal impact — batch verification has fuller cross-finding context

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:19:23 +08:00
wenshao
08a797cf76 fix(review): address 4 Copilot comments
- Add model attribution to no-findings LGTM path
- Handle empty string from getModel() with .trim() || 'unknown'
- Add tests for {{model}} with args and empty model ID
- Fix doc contradiction: PR autofix pushes automatically from worktree

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:13:23 +08:00
wenshao
a5cc2c38cb fix(review): fix 5 worktree issues found in audit
1. Remove gh pr checkout --detach (modifies working tree, defeats
   worktree purpose). Use git fetch only.
2. Add dependency installation step (npm ci etc.) in worktree —
   without it, all TS/JS linting/building fails.
3. Cache and reports written to main project dir, not worktree
   (would be deleted in Step 5).
4. "fix these issues" tip only for local reviews — worktree is
   cleaned up after PR review, so interactive fixing not possible.
5. Autofix push uses explicit remote branch name from Step 1.
6. Move incremental check before dependency install to avoid
   wasting time when no new changes.
7. Fix Step 3 reference: "from Steps 2.5 and 2.6" (includes
   reverse audit findings).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:11:16 +08:00
wenshao
dd2de17de5 feat(review): use ephemeral worktree for PR reviews
Replace the stash + checkout + restore flow with an isolated git
worktree for PR reviews. This eliminates:
- Stash orphan risks (multiple early exit paths)
- Wrong-branch risks (Step 5 restore failures)
- Build cache pollution (worktree has its own state)
- All stash-related error handling complexity

New flow:
- Step 1: git worktree add .qwen/tmp/review-pr-<number>
- All agents operate in the worktree directory
- Autofix commits and pushes from the worktree
- Step 5: git worktree remove (--force for dirty worktrees)

User's working tree is never modified during PR reviews.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 03:07:14 +08:00
wenshao
e502deee28 fix(review): add ms values to Step 1.5 timeout specification
run_shell_command expects timeout in milliseconds. Without explicit
ms values, implementations may pass 120/60 and time out immediately.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 02:58:45 +08:00
wenshao
5effbb696f feat(review): read existing PR comments to avoid duplicate feedback
For PR reviews, fetch existing inline and general comments via gh api
before launching agents. A summary of already-discussed issues is
passed to agents so they don't re-report problems that humans or other
tools have already flagged.

Added to Exclusion Criteria: "Issues already discussed in existing
PR comments."

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 01:07:35 +08:00
wenshao
ba8a3a741f fix(review): fix section numbering, nav entry, and step hierarchy
- commands.md: renumber 1.6→1.7→1.8→1.9 after inserting 1.5 Built-in Skills
- SKILL.md: promote Reverse audit from ### to ## Step 2.6 for consistent
  step hierarchy
- _meta.ts: add code-review to Features navigation sidebar

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 01:04:21 +08:00
wenshao
757bd9865a feat(review): model-aware incremental cache for cross-model review
The incremental review cache now stores modelId alongside commitSha.
When the same PR is re-reviewed with a different model:
- Cache detects model change → runs full review (not skipped)
- Informs user: "Previous review used X. Running full review with Y
  for a second opinion."

Same SHA + same model still skips as before.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 01:01:52 +08:00
wenshao
50d25733d7 feat(review): add reverse audit step to find coverage gaps
Add Step 2.6: after all findings are verified and aggregated, a single
reverse audit agent reviews the diff with full knowledge of what was
already found, specifically looking for important issues that all
previous agents missed.

- Only reports Critical/Suggestion level gaps (not Nice to have)
- Findings go through the same verification as other agents
- Single agent call — minimal cost overhead
- If nothing is found, initial review had strong coverage

This formalizes the "multi-round undirected audit" pattern that proved
effective during the development of this PR (14 rounds, 40+ issues).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 00:56:08 +08:00
wenshao
95a62da039 docs: fix review doc accuracy and remove non-existent /simplify
code-review.md:
- Add PR URL support to Quick Start
- Add "no changes" behavior note
- Fix copilot-instructions.md precedence (prefer .github/, not both)
- Fix "automatically gitignored" → user must ensure .gitignore coverage
- Clarify reports directory is project-relative
- Add "What's NOT Flagged" section (exclusion criteria)

commands.md:
- Replace non-existent /simplify with actual bundled skills
  (/loop, /qc-helper)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 00:50:26 +08:00
wenshao
ac179b0e02 docs: add /review user documentation
Add comprehensive user documentation for the /review command covering:
- Quick start examples for all modes (local, PR, file, --comment)
- Pipeline overview with all steps explained
- Review agents table (5 agents + their focus areas)
- Deterministic analysis (supported languages and tools)
- Severity levels and PR comment filtering rules
- Autofix workflow
- PR inline comments (what gets posted vs terminal-only)
- Follow-up actions (fix/post comments/commit)
- Project review rules (.qwen/review-rules.md etc.)
- Incremental review and caching
- Review report persistence
- Cross-file impact analysis
- Design philosophy

Also add /review and /simplify to the commands reference page
under a new "Built-in Skills" section with link to full docs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 00:47:31 +08:00