Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions internal/scaffold/fullsend-repo/skills/code-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ dimension carry over to another — each requires its own scrutiny.
(e.g., what happens if a critical sub-component fails — does the
caller degrade gracefully or silently proceed?). Trace the full path
from where the mechanism is set to where it is read.
- **Silent-failure severity escalation:** When a code path silently

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] internal-consistency

The SKILL.md silent-failure bullet covers "silently produces wrong results or skips critical work" while the authoritative correctness.md version covers "silently produces wrong results, returns stale or incorrect data, or skips critical work." The omission of "returns stale or incorrect data" narrows the SKILL.md rule: a standalone code review using SKILL.md could rate a stale-data-without-error-signal finding as low, while the same finding in a PR review (using correctness.md) would be escalated to medium or higher. Since stale data returned silently is precisely the class of bug this PR targets (the annotated-tag issue returned a stale/incorrect SHA with no error), the SKILL.md version should include the same qualifying condition.

Suggested fix: Add "returns stale or incorrect data" to the SKILL.md bullet so it reads: "When a code path silently produces wrong results, returns stale or incorrect data, or skips critical work without any error signal, rate the finding medium or higher..."

produces wrong results or skips critical work without any error signal,
rate the finding **medium or higher** — regardless of how unlikely the
triggering condition seems. Severity reflects the failure mode (silent,
total, no error signal), not the probability of triggering.
- Test adequacy: are the right behaviors tested?
- Do the tests actually constrain the code's behavior, or do they
merely assert it runs?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,13 @@ For each finding:
- "Missing test" when the test exists in a different file
2. **Assess severity calibration.** Is the severity proportionate to
the actual risk? Downgrade findings whose severity is inflated
relative to the codebase context.
relative to the codebase context. **Exception — silent-failure
findings:** Do not downgrade findings where the failure mode is
silent (code appears to succeed but produces wrong results with no
error signal). The severity of silent failures reflects the failure
mode, not the probability of triggering. Probability-based reasoning
("X is likely Y, so low impact") is not grounds for downgrading a
silent-failure finding.
3. **Identify duplicates.** Findings from different dimensions that
describe the same underlying issue should be merged. Keep the
higher severity and the more specific remediation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,24 @@ or inter-component contract in the diff:
unavailable, does the caller handle it or silently proceed as if it
succeeded?

### Silent-failure severity escalation

When a code path silently produces wrong results, returns stale or
incorrect data, or skips critical work **without any error signal** to
the caller, rate the finding **medium or higher** — regardless of how
unlikely the triggering condition appears under current usage.

Severity reflects the **failure mode** (silent, total, no error signal),
not the **probability of triggering**. A bug that silently corrupts data
for all affected users is more severe than a bug that crashes loudly for
some, regardless of which is more likely today.

Anti-pattern — probability-based severity discounting: "Low impact since
X is likely Y" is not a valid severity justification when the failure
mode is silent and total. The defensive-coding principle applies: code
must handle all valid inputs correctly, not just the currently expected
ones.

**Consumer completeness:** If the diff adds new values to an enum,
dispatch table, JSON schema enum, or case/switch structure, identify all
code paths that consume or branch on that type (including scripts,
Expand Down
Loading