Skip to content

fix(results): hard-remove legacy grading.json compat reads in manifest.ts - #1711

Merged
christso merged 1 commit into
mainfrom
fix/hard-remove-legacy-grading
Jul 7, 2026
Merged

fix(results): hard-remove legacy grading.json compat reads in manifest.ts#1711
christso merged 1 commit into
mainfrom
fix/hard-remove-legacy-grading

Conversation

@christso

@christso christso commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Found while auditing av-kfik.46.4/av-kfik.46.5 (result-artifact contract cleanup). apps/cli/src/commands/results/manifest.ts still tolerantly read a pre-major-version grading.json shape:

  • assertion_results as a fallback for assertion detail
  • verdict === 'pass' / score-threshold guessing as a fallback for deriving pass
  • graders/evaluators as legacy aliases for component_results — the evaluators branch was explicitly commented "evaluators was renamed to graders in v4.13 — read both for backwards compat... TODO: remove... once old run directories are no longer in use."

Meanwhile apps/cli/src/commands/results/validate.ts already treats all of these same field names (assertion_results, assertions, passed, evidence, verdict, graders, checks) as hard validation errors. One code path rejected the legacy shape as invalid; another silently read it. Since this major version has no production users yet, there's no on-disk bundle to preserve compatibility for — hard-removing is correct rather than adding test coverage for a fallback that will never be exercised.

Changes

  • Removed readGradingAssertionResults()/mapGradingAssertions() — the assertion_results fallback. Call site now derives directly from the grading component itself (mapComponentAssertion), matching what already happens when component_results is present.
  • Removed the graders/evaluators branches in readNestedGradingScores() and the ad hoc type augmentation in hydrateManifestRecord() — both now read only component_results (and the separate, still-current scores field).
  • Simplified mapGradingEvaluator()'s pass derivation to evaluator.pass === truepass is always present per the current grading.json contract, so the verdict/score-threshold fallback was dead weight.

Dashboard's EvalDetail.test.ts already has a test asserting these exact legacy fields (assertion_results, evidence, graders, checks) are ignored when parsing a grading artifact — that code path (parseGradingArtifact/parseComponentResult) was already written correctly with no legacy tolerance, so no changes needed there. No test anywhere exercised manifest.ts's now-removed fallback branches (confirmed via search) — nothing to update/remove on the test side beyond the source itself.

Test plan

  • bun run typecheck (workspace) — clean.
  • bunx biome check on the touched file — clean.
  • bun test apps/cli/test/commands/results — 252/252 pass.
  • bun test apps/dashboard/src/components/EvalDetail.test.ts — 5/5 pass (unaffected, confirms Dashboard-side parsing was already correct).
  • Live check: ran a real eval (examples/features/rubric/evals/operators.eval.yaml, live Azure target + grader), then results summary/results show against the resulting bundle — both render correctly through the simplified code path (assertions with text/passed/evidence, correct scores).

🤖 Generated with Claude Code

…t.ts

Found while auditing av-kfik.46.4/46.5. manifest.ts still tolerantly read a
pre-major-version grading.json shape (assertion_results, verdict-as-pass,
graders/evaluators as aliases for component_results) — while validate.ts
already treats those same field names as hard errors. One code path rejected
the legacy shape, another silently accepted it.

No production users exist yet for this still-unreleased major version, so
there's no on-disk bundle to preserve compatibility for. Removed rather than
tested:
- readGradingAssertionResults()/mapGradingAssertions() (assertion_results
  fallback) — dead, call site now derives directly from the component itself.
- graders/evaluators branches in readNestedGradingScores() and
  hydrateManifestRecord()'s type augmentation — the `evaluators` fallback was
  explicitly commented as a v4.13-era TODO to remove once old run directories
  are gone.
- verdict/score-threshold fallback for deriving `pass` — `pass` is always
  present per the current grading.json contract.

Verified: 252 tests across apps/cli/test/commands/results pass; Dashboard's
EvalDetail.test.ts already asserts these exact legacy fields are ignored
(unaffected, no changes needed there — it was already written correctly).
Live check: real eval run -> `results summary`/`results show` against the
resulting bundle render correctly through the simplified code path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying agentv with  Cloudflare Pages  Cloudflare Pages

Latest commit: b33c2d3
Status: ✅  Deploy successful!
Preview URL: https://900ed169.agentv.pages.dev
Branch Preview URL: https://fix-hard-remove-legacy-gradi.agentv.pages.dev

View logs

@christso
christso merged commit 2e26588 into main Jul 7, 2026
8 checks passed
@christso
christso deleted the fix/hard-remove-legacy-grading branch July 7, 2026 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant