[chubirka]

Plan "traust runs on core development" (11-consumer-adoption/done/15-michele-traust-runs-on-core-development.md), decision 10: the benchmark/scorecards slice. It follows the critical-misses chain (contracts#119 → core#379 → traust#376). This PR is the contract; core's view and traust's publish follow.

views_schema/v1/benchmark-scorecards.schema.json is the ViewModel for traust-core's benchmark-scorecards view over DetectionQualityService.scorecards:

• A scorecards array with one scorecard per auditor version over the recorded benchmark audits, oldest first, empty when none is recorded.
• Each scorecard carries:
  • overall recall, and recall by CWE, severity, provenance, language and split;
  • near misses (review-tier matches, for adjudication, never counted as detected);
  • the injection summary;
  • mean run-to-run stability;
  • targets scored and skipped;
  • its auditor versions.
• A recall is detected over expected, and null when nothing was expected (never 0). Rates stay in [0, 1] and counts stay non-negative.
• Held-out targets are release scoring and never appear in this view. Scoring a directory of unrecorded runs (--runs-dir) remains a core door, not a view.

I validated a real core Scorecard.model_dump(mode="json") against the schema with no errors.

Tests

New tests/test_benchmark_scorecards_view.py: a valid document, an empty list, null recall and unmeasured values, range and non-negative bounds, every field required, and unknown fields refused. Contracts suite: 1,392 passed (the skipped cases are Postgres-only; this PR has no DDL).

🤖 Generated with Claude Code

[qodo-for-traust-sci]

PR Summary by Qodo

Define the benchmark scorecards ViewModel contract

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Define a scorecards view contract for recorded benchmark audits, with recall breakdowns and
 quality metrics.
• Keep held-out release scoring and unrecorded-run scoring outside the view.
• Document the contract and test empty results, nullable measurements, required fields, and bounds.

Diagram

graph TD
  A["Recorded audits"] --> B["Quality service"] --> C["Core view"] --> D["Traust publisher"]
  E["Schema tests"] --> F["Scorecards schema"] --> C

Loading

High-Level Assessment

Keep the data-only JSON Schema alongside the existing Insight view contract. A persisted projection or a contracts-owned typed model would add coupling without serving this ephemeral view; Core can own its typed ViewModel and semantic checks.

Files changed (3) +227 / -0

Enhancement (1) +89 / -0

benchmark-scorecards.schema.jsonAdd the benchmark scorecards ViewModel schema +89/-0

Add the benchmark scorecards ViewModel schema

• Defines a strict scorecards array with overall and dimension-level recall, near misses, injection results, stability, target counts, and auditor versions. Constrains rates and counts while allowing null for unmeasured rates.

views_schema/v1/benchmark-scorecards.schema.json

Tests (1) +126 / -0

test_benchmark_scorecards_view.pyTest the scorecards schema contract +126/-0

Test the scorecards schema contract

• Checks representative and empty documents, nullable measurements, rate and count bounds, required scorecard fields, and rejection of unknown fields.

tests/test_benchmark_scorecards_view.py

Documentation (1) +12 / -0

README.mdDocument benchmark scorecards view semantics +12/-0

Document benchmark scorecards view semantics

• Explains the scorecard contents, empty and null recall behavior, and recorded-audit source. Clarifies that held-out release scoring and unrecorded-run scoring remain outside this view.

README.md

[qodo-for-traust-sci]

Code Review by Qodo

View medium (1)

1. Invalid CWE labels pass scorecard checks 🐞 Bug ⭐ New

Description

The by_cwe property uses the unrestricted breakdown definition instead of applying the CWE
pattern required for expected benchmark findings. A scorecard containing a bucket named not-a-cwe
therefore passes validation, allowing a consumer to treat it as a benchmark CWE category.

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The scorecard schema accepts `by_cwe` keys that cannot occur in the benchmark target's expected CWE vocabulary.

## Fix Focus Areas
- views_schema/v1/benchmark-scorecards.schema.json[90-90]
- tests/test_benchmark_scorecards_view.py[137-150]

## Recommended Fix
Give `by_cwe` a `propertyNames` constraint matching the benchmark-target expected CWE item pattern (`^CWE-\d+$`). Add a regression case showing that a non-CWE bucket is rejected while a valid CWE bucket remains accepted.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. Existing databases lack the new tables 🐞 Bug ≡ Correctness

Description

The new database shape adds remediation tables and run_order.work_item_id without changing the
storage REVISION from 1. An existing database stamped with revision 1 can therefore pass the
unchanged version check while lacking those objects, causing remediation operations and orders that
name a work item to fail.

Code

db/v1/traust_ops/postgres/schema/run_order.sql[39]

+    work_item_id UUID REFERENCES traust.assessment(id) ON DELETE RESTRICT,

Evidence

The repository requires a revision bump and explicit migration instructions for database-shape
changes. The shared revision remains 1, while this PR adds tables and a column; the work-item
upgrade is only returned to callers, which must explicitly inspect and apply it.

CONTRIBUTING.md[36-44]
src/traust_contracts/v1/db.py[15-19]
src/traust_contracts/v1/storage/sql.py[64-68]
src/traust_contracts/v1/ops/sql.py[57-64]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Existing revision-1 databases do not acquire the new column or remediation tables, but the storage revision remains unchanged.
## Fix Focus Areas
- src/traust_contracts/v1/db.py[15-19]
- db/v1/traust_ops/postgres/schema/run_order.sql[36-40]
- db/v1/traust_ops/postgres/upgrades/run_order_work_item.sql[1-8]
- src/traust_contracts/v1/storage/sql.py[64-68]
## Recommended Fix
Bump the shared storage revision and document an explicit upgrade path for existing databases covering both dialects, the new tables, and the work-item column and guards. Add a regression that starts from the preceding stamped shape and verifies it cannot be mistaken for the current shape before upgrade.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

2. Unrelated audits can become work items ⊘ Outdated

Description

run_order_work_item_of_repository checks the referenced assessment’s repository but not whether it
is a remediation assessment or whether the order’s asset runs a named item. An order for
remediate_finding can name an audit assessment, and an unrelated asset can carry a work-item ID;
the new test accepts the former by using an audit assessment as its example.

---

3. A scorecard can have no auditor version ✓ Resolved

Resolving commit: 68c78de

Resolution reason

Added minItems: 1 to auditor_versions in the scorecard schema and a test asserting an empty
auditor_versions array fails validation.

Description

The auditor_versions property accepts an array of any length, although the view contract describes
one scorecard per auditor version. A scorecard with an empty or multiple-version array validates,
leaving consumers without a single version to associate with that entry.

---

4. Held-out or misspelled buckets still pass the view contract ✓ Resolved

Resolving commit: 7c239f0

Resolution reason

Per-dimension breakdown defs were added: by_split forbids the held_out key, by_provenance restricts
keys to the benchmark-target provenance vocabulary, and by_severity restricts keys to the answer-key
severity vocabulary. Negative tests were added for each case.

Description

The breakdown definition accepts any string as a key through additionalProperties, so
by_split, by_provenance and by_severity take values that the contract and its sibling schemas
rule out. A producer that leaks a held-out split bucket, or emits a provenance outside
self_replay/cve_replay/seeded or a severity outside the severity enum, still validates, so the
guarantee that held-out targets never appear in this view goes unchecked.

---

View medium (1)

5. Empty buckets can show false misses ✓ Resolved

Resolving commit: 7c239f0

Resolution reason

Added if/then conditionals to the recall $def: expected>=1 requires a numeric recall, expected<=0
requires recall null. Added a parametrized regression test rejecting zero recall for nothing
expected and null recall for findings expected, applied to overall and per-CWE buckets.

Description

The recall definition permits a numeric zero when expected is zero and permits null when
expected is positive, despite defining the former as unmeasured. If either value reaches the view,
schema validation accepts it for overall or any breakdown, so a bucket with no expected findings
can appear to have 0% recall and a measured bucket can appear unmeasured.

---

Context sources

✅ Context preparation

  Commit messages: summarized; full text retained for this review

Review mode: Auto: ⚖️ Balanced: New schema/contract change; schemas are owner-prioritized high-risk, though only three files.

Tip of the day

💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 245d8bd ⚖️ Balanced

Results up to commit 0d2566f ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Empty buckets can show false misses ✓ Resolved

Resolving commit: 7c239f0

Resolution reason

Added if/then conditionals to the recall $def: expected>=1 requires a numeric recall, expected<=0
requires recall null. Added a parametrized regression test rejecting zero recall for nothing
expected and null recall for findings expected, applied to overall and per-CWE buckets.

Description

The recall definition permits a numeric zero when expected is zero and permits null when
expected is positive, despite defining the former as unmeasured. If either value reaches the view,
schema validation accepts it for overall or any breakdown, so a bucket with no expected findings
can appear to have 0% recall and a measured bucket can appear unmeasured.

---

Results up to commit 4b1a274 🧠 Deep

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Existing databases lack the new tables 🐞 Bug ≡ Correctness

Description

The new database shape adds remediation tables and run_order.work_item_id without changing the
storage REVISION from 1. An existing database stamped with revision 1 can therefore pass the
unchanged version check while lacking those objects, causing remediation operations and orders that
name a work item to fail.

Code

db/v1/traust_ops/postgres/schema/run_order.sql[39]

+    work_item_id UUID REFERENCES traust.assessment(id) ON DELETE RESTRICT,

Evidence

The repository requires a revision bump and explicit migration instructions for database-shape
changes. The shared revision remains 1, while this PR adds tables and a column; the work-item
upgrade is only returned to callers, which must explicitly inspect and apply it.

CONTRIBUTING.md[36-44]
src/traust_contracts/v1/db.py[15-19]
src/traust_contracts/v1/storage/sql.py[64-68]
src/traust_contracts/v1/ops/sql.py[57-64]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Existing revision-1 databases do not acquire the new column or remediation tables, but the storage revision remains unchanged.
## Fix Focus Areas
- src/traust_contracts/v1/db.py[15-19]
- db/v1/traust_ops/postgres/schema/run_order.sql[36-40]
- db/v1/traust_ops/postgres/upgrades/run_order_work_item.sql[1-8]
- src/traust_contracts/v1/storage/sql.py[64-68]
## Recommended Fix
Bump the shared storage revision and document an explicit upgrade path for existing databases covering both dialects, the new tables, and the work-item column and guards. Add a regression that starts from the preceding stamped shape and verifies it cannot be mistaken for the current shape before upgrade.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

2. Unrelated audits can become work items ⊘ Outdated

Description

run_order_work_item_of_repository checks the referenced assessment’s repository but not whether it
is a remediation assessment or whether the order’s asset runs a named item. An order for
remediate_finding can name an audit assessment, and an unrelated asset can carry a work-item ID;
the new test accepts the former by using an audit assessment as its example.

---

3. Held-out or misspelled buckets still pass the view contract ✓ Resolved

Resolving commit: 7c239f0

Resolution reason

Per-dimension breakdown defs were added: by_split forbids the held_out key, by_provenance restricts
keys to the benchmark-target provenance vocabulary, and by_severity restricts keys to the answer-key
severity vocabulary. Negative tests were added for each case.

Description

The breakdown definition accepts any string as a key through additionalProperties, so
by_split, by_provenance and by_severity take values that the contract and its sibling schemas
rule out. A producer that leaks a held-out split bucket, or emits a provenance outside
self_replay/cve_replay/seeded or a severity outside the severity enum, still validates, so the
guarantee that held-out targets never appear in this view goes unchecked.

---

4. A scorecard can have no auditor version ✓ Resolved

Resolving commit: 68c78de

Resolution reason

Added minItems: 1 to auditor_versions in the scorecard schema and a test asserting an empty
auditor_versions array fails validation.

Description

The auditor_versions property accepts an array of any length, although the view contract describes
one scorecard per auditor version. A scorecard with an empty or multiple-version array validates,
leaving consumers without a single version to associate with that entry.

---

Results up to commit 68c78de ⚖️ Balanced

No changes from previous review

Results up to commit 7c239f0 ⚖️ Balanced

No changes from previous review

Results up to commit 9a5157f ⚖️ Balanced

No changes from previous review

Results up to commit 2853e3e 🚀 Fast

No changes from previous review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 4b1a274

[qodo-code-review]

Code review by qodo was updated up to the latest commit 68c78de

[chubirka]

Addressing the Qodo review findings (both the round-1 comment and the current one, reviewed up to 68c78de; 7c239f0 lands the fixes):

Round 2, finding 1 — "Existing databases lack the new tables" (High): obsolete. The cited focus areas (run_order.sql's work_item_id, upgrades/run_order_work_item.sql, the remediation tables, the ops/sql.py export) are no longer in this PR: 005fb8b merged development, which had already absorbed that DB work via the remediation-as-schema-of-record and upgrades-channel-removal changes, and the merge took development's side wholesale. The current diff is exactly three files — README.md, tests/test_benchmark_scorecards_view.py, views_schema/v1/benchmark-scorecards.schema.json — all view contract. No DDL, no REVISION change, so the bump this finding asks for has no object here.

Round 2, finding 2 — "Unrelated audits can become work items": ⊘ Outdated (same reason — the run-order DDL is gone from this PR).

Round 2, finding 3 — "A scorecard can have no auditor version": ✓ fixed in 68c78de (minItems: 1 + regression test), already marked resolved.

Round 2, finding 4 — "Held-out or misspelled buckets still pass the view contract": fixed in 7c239f0. by_provenance and by_severity now propertyNames-$ref the benchmark-target schema's provenance and severity vocabularies (same pattern as compliance-precision's framework id: validators must provide a local artifact-schema registry, not fetch over the network), and by_split refuses the held_out bucket via propertyNames: {not: {const: "held_out"}}. Split keys are only constrained negatively because core emits "development"/"held_out" literally and this view excludes the latter by contract — a closed "development"-only enum would also forbid nothing core can legitimately emit today, but the negative form states the actual guarantee. Verified against a real Scorecard.model_dump(mode="json") built from core's models: a normal run validates; a run that includes held-out targets is refused with 'held_out' should not be valid under {'const': 'held_out'}. Negative tests cover by_split: held_out, an out-of-enum provenance and an out-of-enum severity.

Round 2, finding 5 / round 1, finding 3 — "Empty buckets can show false misses" (recall/expected disconnect): fixed in 7c239f0. The recall def now carries two if/then conditionals: expected >= 1 requires recall to be a number, expected == 0 requires recall to be null. Both contradictory combinations are rejected on overall and on a breakdown bucket; a measured zero (expected: 1, recall: 0.0) still validates.

Round 1, finding 1 — "Impossible injection totals pass validation": kept as documented boundary. This matches the sibling compliance-precision contract's explicit "JSON Schema checks structure and ranges, not count arithmetic" rule, which the README section for this view already follows. passed > total (and detected > expected) are producer invariants (passed = sum of per-canary booleans over total = len(canaries)), not schema bounds; encoding one comparison and not the rest would be arbitrary. Not changed.

Round 1, finding 2 — "Reviewers cannot identify near misses": clarified, deliberately summary-only. This view is one scorecard per auditor version over every recorded audit — a fleet-level summary. The per-match identities for adjudication are the run's Expected-finding matches table (core's benchmark_markdown renders target/CWE/severity/candidate per review-tier match), which is a different, per-run surface. The near_misses description now states this explicitly instead of implying a list that isn't there. Not changed structurally.

make lint-fix, make test (1479 passed, 1 skipped, live Postgres), ci/enum_registry.py --check --enforce and ci/schema_diff.py (0 changes core must absorb) all pass on 7c239f0.

[qodo-code-review]

Code review by qodo was updated up to the latest commit 7c239f0

[smith-xyz]

first pass — 5 notes

[smith-xyz]

pass_rate is nullable with no stated null condition, unlike recall, which pins null exactly when nothing was expected. Add the same if/then pair on total, or a description saying when it is null.

[smith-xyz]

"Severest first" is unenforceable — object keys have no order. If severity ordering matters for rendering, make by_severity an ordered array; otherwise drop the phrase.

[smith-xyz]

Any key except held_out validates (typos included), unlike by_provenance/by_severity, which pin recorded vocabularies. Pin the producer's split vocabulary, or state in the description that keys are producer-defined free-form.

[smith-xyz]

Duplicate versions validate today — add "uniqueItems": true beside minItems.

[smith-xyz]

"Listed for adjudication" reads as a list, but the schema carries only a count. Reword to "counted; identities live in the run's Expected-finding matches table" to match the near_misses description.

[chubirka]

First-pass notes, all five addressed in 097d789 (development merged after in 9a5157f, so this head now carries the fold and the pqc-worklist schema alongside this view — core #393 can pin it once this merges):

• pass_rate: same if/then coupling as recall — null exactly when total is 0, never a measured 0 or an unmeasured number. Tested both directions.
• by_severity: dropped the unenforceable phrase; the description now says the producer orders keys severest first while object keys carry no order of their own.
• by_split: pinned to the recorded vocabulary (development only); a typo key is refused like by_provenance/by_severity.
• auditor_versions: uniqueItems added beside minItems.
• README: near misses are counted, identities live in the run's Expected-finding matches table.

[qodo-code-review]

Code review by qodo was updated up to the latest commit 9a5157f

[qodo-code-review]

Code review by qodo was updated up to the latest commit 2853e3e

[chubirka]

Follow-up in 2853e3e: the severity note is now fully integrated — the severest first phrasing is dropped entirely rather than reworded; by_severity's description just says keys are unordered. The other four notes were already addressed in 097d789 (pass_rate if/then on total, by_split pinned to development, uniqueItems, README near-miss rewording). Contracts suite: 1571 passed, 1 skipped; ruff clean.

[chubirka]

/lgtm

[openshift-ci]

@chubirka: you cannot LGTM your own PR.

Details

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chubirka

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [chubirka]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[qodo-code-review]

Code review by qodo was updated up to the latest commit 245d8bd