Skip to content

Add severity to legacy findings #135

Description

@williamthorsen

Problem

All legacy findings use a flat L{n} category with criticality excluded, giving readers no indication of severity. A review might surface a legacy security bug and a legacy naming nit, but both appear as undifferentiated L findings. Readers cannot quickly triage which legacy issues are worth addressing.

Context

The finding scheme (F/W/T/R/S/L) is defined in review-criteria and referenced across multiple surfaces:

Surface Role
review-criteria/SKILL.md Canonical definition: ID scheme, category criteria, criticality mapping, escalation rules
review-change/SKILL.md Issue numbering list + output templates (branch and commit scope)
orchestrated-reviewer.md Finding format example with **Severity:** legacy
respond-to-review/SKILL.md Disposition format (ACCEPT/REJECT only for legacy)
artifact-conventions.md Reference copy of the finding scheme table + category criteria

Aspect reviewers (aspect-code-reviewer, aspect-test-reviewer, aspect-silent-failure-reviewer, code-simplification-reviewer) reference review-criteria but don't define their own legacy format — they inherit the change automatically.

The orchestrated-coder has no awareness of legacy findings and is unaffected.

Solution

Replace the flat L{n} category with severity-tagged legacy IDs that reuse the existing severity letters: F{n}-L, W{n}-L, T{n}-L, R{n}-L, S{n}-L.

Design decisions

  • Shared counter per severity letter. Legacy findings share the same numbering sequence as author findings. If a review has F1, F2 (author findings), the first legacy FIXME is F3-L. Every ID is globally unique within the review.
  • Legacy section stays separate. Legacy findings remain grouped under "Legacy observations" in review output — they are not inlined into the severity sections. Attention to legacy findings is discretionary.
  • Criticality stays excluded. Legacy findings do not participate in criticality rollup, merge-blocking, or flow control. This is a reader-facing change only.
  • No escalation. The rule "L findings are never escalated" still holds — now expressed as "{n}-L findings are never escalated."
  • Disposition unchanged. respond-to-review keeps ACCEPT/REJECT only for legacy findings, with updated ID format in the template.

Before

#### L1: Hardcoded timeout

- **Severity:** legacy
- **Location:** `src/api/client.ts:42`
- **Description:** {pre-existing issue observed}
- **Recommendation:** {future opportunity}

After

#### F3-L: Hardcoded timeout

- **Severity:** critical (legacy)
- **Location:** `src/api/client.ts:42`
- **Description:** {pre-existing issue observed}
- **Recommendation:** {future opportunity}

Acceptance criteria

  • review-criteria/SKILL.md: finding scheme table replaces L{n} row with {F,W,T,R,S}{n}-L convention; legacy category criteria updated; escalation rule updated; criticality mapping and PR comment format table updated
  • review-change/SKILL.md: issue numbering list and "Legacy observations" section in both output templates updated to show severity-tagged IDs
  • orchestrated-reviewer.md: finding format example updated from L1 / **Severity:** legacy to severity-tagged format
  • respond-to-review/SKILL.md: legacy disposition template updated with severity-tagged ID example
  • artifact-conventions.md: reference copy of finding scheme table and legacy category criteria updated

Metadata

Metadata

Labels

featureAdded or improved external functionalityscope:agents

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions