feat(triage-bug): add persistence-impact analysis to Step 3

evals/triage-bug/evals.json

@@ -80,6 +80,20 @@
         "Version resolution offers three options: Keep, Replace, or Augment the existing value (Step 4.5.1)",
         "The skill does NOT silently overwrite or skip the existing value — it waits for user input (Step 4.5.1)"
       ]
+    },
+    {
+      "id": 7,
+      "prompt": "Triage Bug issue ACME-520. The issue details are in bug-issue-persistence-impact.md, the bug template is in bug-template-mock.md, the repository context is in repo-context-persistence-mock.md, and the project CLAUDE.md is in claude-md-bug-config.md. Do NOT actually call Jira MCP, Serena, or any external tools. Instead, write your triage analysis to the workspace outputs/ directory: write outputs/bug-parsing.md with the parsed description sections from Step 1, write outputs/investigation.md with the codebase investigation findings from Steps 2-3 including the persistence-impact analysis, write outputs/root-cause.md with the root cause analysis from Step 4, and write outputs/task-description.md with the full Task description you would create in Step 5 (following shared/task-description-template.md format). At the top of outputs/task-description.md, before the task description body, include a Jira API metadata block showing the parameters you would pass to jira.create_issue: project key, issue type, and labels (including ai-generated-jira).",
+      "expected_output": "A complete triage demonstrating: correct Bug description parsing, codebase investigation that traces compute_risk_score() output through create_assessment() to the assessments table (persistence boundary found), and a Task description that includes both a code fix AND a data migration. The Task's Files to Create must include a migration file following the Diesel convention (YYYY-MM-DD-NNNNNN_description/up.sql). The Implementation Notes must describe the migration logic for correcting existing risk_score values in the assessments table.",
+      "files": ["files/bug-issue-persistence-impact.md", "files/bug-template-mock.md", "files/repo-context-persistence-mock.md", "files/claude-md-bug-config.md"],
+      "assertions": [
+        "Investigation traces compute_risk_score() output through create_assessment() to diesel::insert_into(assessments::table) and identifies the persistence boundary at the assessments table, risk_score column (Step 3 persistence-impact analysis)",
+        "Investigation records that risk_score is written at ingestion time (assessment creation) and is NOT recomputed on read, confirming a data migration is needed (Step 3 persistence-impact analysis)",
+        "Task description Files to Create includes a data migration file following the Diesel naming convention discovered in the repository (YYYY-MM-DD-NNNNNN_description/up.sql) (Step 5 data migration)",
+        "Task description Implementation Notes includes migration logic to recompute and update existing risk_score values in the assessments table (Step 5 data migration)",
+        "Task description Acceptance Criteria includes a criterion that existing assessments with incorrect persisted risk_score values are corrected by the migration (Step 5 data migration)",
+        "Task description has reproducer test as the FIRST acceptance criterion — before the data migration criterion and any fix criteria (Step 5 Acceptance Criteria ordering)"
+      ]
     }
   ]
 }

evals/triage-bug/files/bug-issue-persistence-impact.md

@@ -0,0 +1,45 @@
+<!-- SYNTHETIC TEST DATA — Bug issue where buggy output is persisted to database, for triage-bug persistence-impact eval testing -->
+
+# Mock Jira Bug Issue
+
+**Key**: ACME-520
+**Summary**: Risk scores are computed with wrong denominator, producing inflated values
+**Issue Type**: Bug (ID: 10020)
+**Status**: New
+**Labels**: reported-by-user
+**Component**: risk-engine
+**Affects Version/s**: (none)
+**Web URL**: https://mock-jira.example.com/browse/ACME-520
+
+---
+
+## Description
+
+### **Issue Description**
+
+The `compute_risk_score()` function in the risk engine divides by total dependencies
+instead of vulnerable dependencies, producing inflated risk scores for all assessments.
+
+### **Steps to Reproduce**
+
+1. Ingest an SBOM with 100 total dependencies, 5 of which are vulnerable.
+2. Create a risk assessment for the ingested SBOM.
+3. Retrieve the risk assessment via `GET /api/v2/assessments/{id}`.
+4. Inspect the `risk_score` field.
+
+### **Expected Result**
+
+The risk score should be `5 / 100 = 0.05` (vulnerable / total).
+
+### **Actual Result**
+
+The risk score is `100 / 5 = 20.0` (total / vulnerable). The numerator and
+denominator are swapped.
+
+### **Environment / Version**
+
+Not specified.
+
+### **Attachments**
+
+None.

evals/triage-bug/files/repo-context-persistence-mock.md

@@ -0,0 +1,123 @@
+<!-- SYNTHETIC TEST DATA — mock repository context with a persistence chain for triage-bug persistence-impact eval testing -->
+
+# Mock Repository Context: acme-backend (persistence scenario)
+
+This file simulates the relevant code paths that the triage-bug skill would
+discover during Step 3 (Codebase Investigation) for bug ACME-520.
+
+## File: modules/risk/src/score.rs
+
+### compute_risk_score (the buggy function)
+
+```rust
+/// Computes a risk score for an SBOM based on its vulnerability profile.
+pub fn compute_risk_score(total_deps: u32, vulnerable_deps: u32) -> f64 {
+    // BUG: numerator and denominator are swapped
+    total_deps as f64 / vulnerable_deps as f64
+}
+```
+
+**Root cause**: The division operands are reversed. Should be
+`vulnerable_deps / total_deps`, not `total_deps / vulnerable_deps`.
+
+## File: modules/risk/src/assessment.rs
+
+### create_assessment (caller that persists the result)
+
+```rust
+use crate::score::compute_risk_score;
+use diesel::prelude::*;
+
+/// Creates a risk assessment for an SBOM and persists it to the database.
+pub fn create_assessment(
+    conn: &mut PgConnection,
+    sbom_id: i64,
+    total_deps: u32,
+    vulnerable_deps: u32,
+) -> Result<Assessment, Error> {
+    let score = compute_risk_score(total_deps, vulnerable_deps);
+
+    diesel::insert_into(assessments::table)
+        .values((
+            assessments::sbom_id.eq(sbom_id),
+            assessments::risk_score.eq(score),    // <-- persisted here
+            assessments::created_at.eq(now),
+        ))
+        .get_result(conn)
+}
+```
+
+**Persistence boundary**: The risk score computed by `compute_risk_score()` is
+written to the `assessments` table, `risk_score` column, at ingestion time
+(when the assessment is first created). It is NOT recomputed on read.
+
+## File: modules/risk/src/endpoints.rs
+
+### GET /api/v2/assessments/{id} (query endpoint)
+
+```rust
+/// Retrieves a risk assessment by ID.
+pub async fn get_assessment(
+    path: web::Path<i64>,
+    db: web::Data<DbPool>,
+) -> Result<HttpResponse, Error> {
+    let assessment = assessments::table
+        .find(path.into_inner())
+        .first::<Assessment>(&mut db.get()?)?;
+
+    Ok(HttpResponse::Ok().json(assessment))
+}
+```
+
+**Note**: This endpoint reads the persisted `risk_score` directly from the
+database — it does NOT recompute the score. Fixing `compute_risk_score()` alone
+will only correct future assessments; existing assessments retain the wrong score.
+
+## Database schema
+
+### Table: assessments
+
+| Column | Type | Description |
+|--------|------|-------------|
+| id | BIGSERIAL | Primary key |
+| sbom_id | BIGINT | Foreign key to sboms table |
+| risk_score | DOUBLE PRECISION | Computed risk score (persisted at creation) |
+| created_at | TIMESTAMPTZ | Creation timestamp |
+
+## Existing migrations
+
+### Directory: migration/
+
+```
+migration/
+├── 2024-01-15-000001_create_sboms/
+│   ├── up.sql
+│   └── down.sql
+├── 2024-02-20-000002_create_assessments/
+│   ├── up.sql
+│   └── down.sql
+└── 2024-03-10-000003_add_severity_column/
+    ├── up.sql
+    └── down.sql
+```
+
+Migration files follow the Diesel convention: `YYYY-MM-DD-NNNNNN_description/up.sql`.
+
+## Test files
+
+### Existing test: modules/risk/tests/score_test.rs
+
+```rust
+#[test]
+fn test_risk_score_all_vulnerable() {
+    let score = compute_risk_score(10, 10);
+    assert_eq!(score, 1.0);
+}
+```
+
+**Note**: This test passes even with the bug because `10 / 10 = 1.0` regardless
+of operand order. No test exercises the case where `total != vulnerable`.
+
+## CONVENTIONS.md
+
+The repository does not have a CONVENTIONS.md at its root.

plugins/sdlc-workflow/skills/triage-bug/SKILL.md

@@ -315,6 +315,35 @@ to inform the generated task's Implementation Notes.
 - Search for existing test files and patterns relevant to writing the reproducer test
 - Identify reusable utilities, helpers, or shared modules relevant to the fix
 
+### Persistence-impact analysis
+
+After identifying the buggy function(s), determine whether their output is persisted
+to the database at ingestion time or computed at query time. Fixing the code alone
+corrects future data, but if incorrect values were already written to the database,
+existing records remain stale — a data migration is needed to correct them.
+
+1. **Trace output to persistence boundary**: starting from the buggy function's
+   return value, use `find_referencing_symbols` (or Grep as fallback) to follow
+   all callers up the call chain. At each hop, check whether the caller writes
+   the value to a database (e.g., `insert`, `update`, `save`, `persist`, ORM
+   model creation, or raw SQL `INSERT`/`UPDATE` statements).
+2. **If a persistence boundary is found**: record the following and carry them
+   forward to Step 5 (Generate Task):
+   - The table name and column where the value is stored
+   - The write operation location (file and symbol)
+   - Whether the value is written at ingestion time (once, when data first enters
+     the system) or updated on every access — ingestion-time writes are the primary
+     concern, since they produce stale data that is never self-correcting
+3. **If no persistence boundary is found**: the output is computed at query time
+   (e.g., derived on each API request from source data). No data migration is
+   needed — proceed normally.
+
+> **Example trace:**
+>
+> `describing_packages()` → `suppliers()` → `SbomInformation` (struct field)
+> → `ingest_sbom()` → `insert_into(sbom::table).values(suppliers)` — **persistence
+> boundary found** at `sbom` table, `suppliers` column.
+
 ## Step 4 – Root Cause Analysis
 
 Synthesize the findings from Steps 2 and 3 into a root cause narrative.
@@ -499,6 +528,26 @@ Translate the Steps to Reproduce into test-level guidance for the reproducer. In
 
 Reference the Bug issue key for traceability (e.g., "Fixes PROJ-456").
 
+#### Data migration (when persistence impact is flagged)
+
+When Step 3's persistence-impact analysis found a persistence boundary, the
+generated task must include a data migration to correct existing records alongside
+the code fix. Add the following to the task:
+
+1. **Files to Create**: add a data migration file. To determine the correct
+   file name, location, and format, search the repository for existing migration
+   files using `search_for_pattern` (or Grep as fallback) — look for directories
+   named `migrations/`, `migration/`, or `db/migrate/`, and match the naming
+   convention used by existing migration files (e.g., timestamped filenames,
+   sequential numbering, version-prefixed names).
+2. **Implementation Notes**: describe the migration logic — the table and column
+   to update, the correct value to recompute from source data, and the query or
+   script that performs the correction. Reference the existing migration pattern
+   discovered in the repository so that implement-task follows the established
+   conventions.
+3. **Acceptance Criteria**: add a criterion that existing records with incorrect
+   persisted values are corrected by the migration.
+
 ### Bug Context extension section
 
 Add a **Bug Context** section to the task description, after the standard template