diff --git a/evals/triage-bug/evals.json b/evals/triage-bug/evals.json index 74efba69..fd59b75a 100644 --- a/evals/triage-bug/evals.json +++ b/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)" + ] } ] } diff --git a/evals/triage-bug/files/bug-issue-persistence-impact.md b/evals/triage-bug/files/bug-issue-persistence-impact.md new file mode 100644 index 00000000..4790f333 --- /dev/null +++ b/evals/triage-bug/files/bug-issue-persistence-impact.md @@ -0,0 +1,45 @@ + + +# 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. diff --git a/evals/triage-bug/files/repo-context-persistence-mock.md b/evals/triage-bug/files/repo-context-persistence-mock.md new file mode 100644 index 00000000..0d40571c --- /dev/null +++ b/evals/triage-bug/files/repo-context-persistence-mock.md @@ -0,0 +1,123 @@ + + +# 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 { + 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, + db: web::Data, +) -> Result { + let assessment = assessments::table + .find(path.into_inner()) + .first::(&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. diff --git a/plugins/sdlc-workflow/skills/triage-bug/SKILL.md b/plugins/sdlc-workflow/skills/triage-bug/SKILL.md index 07c535e7..1b2987fd 100644 --- a/plugins/sdlc-workflow/skills/triage-bug/SKILL.md +++ b/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