From c7cabe05b0f32291737a022f82e1862f97930dcf Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Tue, 2 Jun 2026 01:46:31 -0700 Subject: [PATCH] agents|fix: Reframe legacy finding-ID rule to prevent ID collisions Legacy findings in a code review now draw their number from the same per-letter sequence as author findings, assigned before the `-L` marker is appended, so a legacy ID can no longer repeat an author finding's number and a bare reference like `T1` stays unambiguous. --- .../agents/content/skills/_data/artifact-conventions.md | 6 +++--- packages/agents/content/skills/respond-to-review/SKILL.md | 2 +- packages/agents/content/skills/review-branch/SKILL.md | 2 +- packages/agents/content/subagents/orchestrated-reviewer.md | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/agents/content/skills/_data/artifact-conventions.md b/packages/agents/content/skills/_data/artifact-conventions.md index d634434e..61037131 100644 --- a/packages/agents/content/skills/_data/artifact-conventions.md +++ b/packages/agents/content/skills/_data/artifact-conventions.md @@ -793,10 +793,10 @@ Consumers that present or report findings (review skills, wrap-up, response arti - Documentation improvements - **Gate:** Raise only when the change aligns with a codebase convention the code violates, has measurable improvement evidence (perf, correctness, readability with a concrete example), or follows a widely accepted external standard (linter rule, language spec, ecosystem norm with a citation). "Another valid way to write it" does not qualify. -**Legacy (-L suffix)** — pre-existing code observation: +**Legacy (`-L` suffix)** — pre-existing code observation: -- Issues in code not authored in this branch — use the same severity letter as the equivalent author finding plus a `-L` suffix -- Legacy findings share the numbering sequence with author findings of the same severity letter. Example: If a review has `F1`, `F2` (author findings), the first legacy FIXME is `F3-L` +- Issues in code not authored in this branch — use the same severity letter as the equivalent author finding. +- The `-L` is a marker, not part of the ID — the ID is the number in front of it. Assign that number from the shared per-letter sequence as if this were an author finding, then append `-L`. Example: after author findings `F1`, `F2`, the first legacy FIXME is `F3-L`. - Set the `**Severity:**` field to `{severity} (legacy)` — e.g., `critical (legacy)`, `warning (legacy)`, `suggestion (legacy)` - Frame as future opportunities, not current defects - Never count against the review score diff --git a/packages/agents/content/skills/respond-to-review/SKILL.md b/packages/agents/content/skills/respond-to-review/SKILL.md index c13311ec..c6eedb3a 100644 --- a/packages/agents/content/skills/respond-to-review/SKILL.md +++ b/packages/agents/content/skills/respond-to-review/SKILL.md @@ -217,7 +217,7 @@ The body following the frontmatter has this structure: ### Legacy 🔍 -#### F3-L: {title from review} +#### F2-L: {title from review} - **Disposition:** ACCEPT | REJECT - **Rationale:** {reasoning} diff --git a/packages/agents/content/skills/review-branch/SKILL.md b/packages/agents/content/skills/review-branch/SKILL.md index 68a9e05b..54da4582 100644 --- a/packages/agents/content/skills/review-branch/SKILL.md +++ b/packages/agents/content/skills/review-branch/SKILL.md @@ -78,7 +78,7 @@ Uniquely number all issues for easy reference. See [finding scheme](../_data/art - TODOs: `T{n}` — should fix, can wait for next PR - Recommendations: `R{n}` — advisable but discretionary - Suggestions: `S{n}` — optional improvement -- Legacy: `{F,W,T,R,S}{n}-L` — observation in pre-existing code, not authored in this change. Uses the same severity letter as the equivalent author finding plus a `-L` suffix (e.g., `F3-L`, `W2-L`) +- Legacy: `{F,W,T,R,S}{n}-L` — observation in pre-existing code, not authored in this change. Same severity letter as the equivalent author finding; number it from that letter's shared sequence first, then append `-L` as a marker (e.g., after `F1`, `F2`, the first legacy FIXME is `F3-L`) ## Output format diff --git a/packages/agents/content/subagents/orchestrated-reviewer.md b/packages/agents/content/subagents/orchestrated-reviewer.md index ed691235..7f29a7ad 100644 --- a/packages/agents/content/subagents/orchestrated-reviewer.md +++ b/packages/agents/content/subagents/orchestrated-reviewer.md @@ -141,14 +141,14 @@ The finalized form of the review file. See [Incremental review writes](#incremen - **Description:** {Optional improvement} - **Recommendation:** {How to improve} -#### F3-L: {title} +#### F2-L: {title} - **Location:** `src/auth/utils.ts:15` - **Severity:** critical (legacy) - **Description:** {Pre-existing issue observed} - **Recommendation:** {Future opportunity} -{Use the same pattern for all severity letters: `W2-L` with `warning (legacy)`, `T1-L` with `todo (legacy)`, etc.} +{Use the same pattern for all severity letters: `W2-L` with `warning (legacy)`, `T2-L` with `todo (legacy)`, etc.} ``` If no findings, write: