Skip to content

feat(ci): golden-set regression eval for LLM verifier behaviour (#129) - #640

Merged
Weegy merged 2 commits into
byte5ai:mainfrom
sneumannb5:feat/129-golden-set-regression
Aug 11, 2026
Merged

feat(ci): golden-set regression eval for LLM verifier behaviour (#129)#640
Weegy merged 2 commits into
byte5ai:mainfrom
sneumannb5:feat/129-golden-set-regression

Conversation

@sneumannb5

@sneumannb5 sneumannb5 commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

What

Golden-set regression eval for LLM verifier behaviour (Closes #129). A frozen corpus runs through a real VerifierPipeline (real ClaimExtractor + EvidenceJudge on the pinned VERIFIER_MODEL, fixture-backed sources) and fails when a known-good input regresses to a worse verdict class (approved / approved_with_disclaimer / blocked). New golden-eval.yml runs it on main push + dispatch + weekly cron.

Why

CI verifies plumbing (lint / typecheck / unit tests) but never LLM behaviour against the pinned model — a model bump or prompt edit can silently regress agent quality (weakness #13, version drift). Asserting the verdict class rather than the raw string gives a stable signal despite generation stochasticity; a first-sample miss re-runs up to 3× and decides by majority to absorb jitter.

Test plan

  • npm run typecheck:golden (new — puts test/golden/** under tsc) — clean
  • npx tsx --test test/goldenRunner.test.ts test/goldenModel.test.ts22/22 pass
  • Mutation-checked both suites are load-bearing (invert runEntry guard → 3 red; drop knowledgeGraphToolsCalled / toolPostconditionViolations
    in toVerifierInput → wiring tests red; revert → green)
  • Ran shouldTriggerVerifier over the whole corpus — 0 broken judge-dependent fixtures (every disclaimer/contradiction answer now trips a
    trigger signal)
  • npm run eval:golden with a real key
  • Full npm run lint && npm run typecheck && npm run test

Risk / blast radius

  • CI / release tooling: new golden-eval.yml workflow (main push + workflow_dispatch + weekly cron, not per-PR). Skips cleanly with a notice when ANTHROPIC_API_KEY is absent, so an unconfigured repo/fork stays green.
  • New secret required: ANTHROPIC_API_KEY GitHub Actions secret + a cost budget — maintainer action, not yet configured.
  • New npm script typecheck:golden, chained into typecheck (runs per-PR). Test-only; no src/ change, no migration, no public API, no new env-var in the runtime.
  • Default npm test stays green key-free (eval lives outside the test/**/*.test.ts glob).

sneumannb5 and others added 2 commits August 10, 2026 10:29
…5ai#129)

Frozen corpus run through a real VerifierPipeline (real ClaimExtractor +
  EvidenceJudge on the pinned VERIFIER_MODEL, fixture-backed sources) that
  fails when a known-good input regresses to a worse verdict class. Asserts the
  verdict class (approved / approved_with_disclaimer / blocked), the stable
  signal despite generation stochasticity. Flake policy: re-run a first-sample
  miss up to 3x, decide by majority. New golden-eval.yml runs on main push +
  dispatch + weekly cron (not per-PR, cost), gated on ANTHROPIC_API_KEY and
  skips cleanly when absent.

  - Split harness into a pure layer (goldenRunner.ts, type-only verifier import,
    key- and build-free) and a model layer (goldenModel.ts, real pipeline wiring)
  - goldenModel.test.ts drives the real pipeline to blocked/approved key-free via
    a stub LlmProvider, so the wiring is load-bearing in CI, not decorative
  - typecheck:golden (chained into typecheck) puts test/golden under tsc, which
    test/ otherwise escapes; caught pre-existing noUncheckedIndexedAccess bugs
  - Corpus: every judge-dependent answer carries a hard trigger signal, else
    shouldTriggerVerifier skips extraction and the entry silently approves
  - README documents the trigger trap and the add-a-corpus-entry-for-a-new-agent
    procedure; v2 (deterministic source + full-turn eval) tracked in byte5ai#639

  Refs byte5ai#130, byte5ai#131, byte5ai#132. Follow-up byte5ai#639.
@Weegy

Weegy commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Merge prep + blast-radius review

Merged current main into this branch and re-verified locally, because the green checks on this PR were earned on 2026-08-10 against 6ebcaf998 commits stale by now. Merge commit f1f6b2a5, zero conflicts.

Local verification (Node 22.22.3, merged branch)

Step Result
npm ci exit 0
npm run build exit 0
npm run lint exit 0
npm run typecheck (incl. new typecheck:golden) exit 0
npm test 6139 tests · 6135 pass · 0 fail · 4 skipped

All six golden describe blocks pass inside the default suite, key-free, as designed.

Real blast radius

The headline +1165 / 14 files overstates it. Executable surface is 810 lines of TS across 5 files, plus an 86-line workflow, a 16-line tsconfig and 2 script lines; the remaining ~250 lines are the README and 13 JSONL fixtures.

  • No src/ change, no migration, no public API, no new runtime env var. Confirmed.
  • middleware/package.json: the PR's own delta is exactly typecheck:golden + eval:golden. (Diffing the pre-merge branch against today's main showed dependency downgrades — better-sqlite3 13→12, cookie 2→0.7, openai 7→6 — but that was purely branch staleness, and the merge resolved it back to main's versions.)
  • typecheck:golden is chained into typecheck, which does run per-PR. It needs the workspace dist/; ci.yml's middleware job runs npm run build before Typecheck, so the prerequisite holds. Verified in the workflow, not assumed.
  • golden-eval.yml uses actions/checkout@v7 / setup-node@v7, matching the existing ci.yml convention.
  • The .eval.ts extension correctly sits outside the test/**/*.test.ts glob, so npm test stays key-free.

The module split is the strongest part of the design: goldenRunner.ts keeps its only @omadia/verifier reference as an import type (erased at runtime), so the pure suite genuinely loads without dist/, while goldenModel.ts quarantines the value import. Splitting at the module boundary rather than the function boundary is what makes that claim true rather than aspirational. The control case in goldenModel.test.ts ("does NOT block a KG answer that carries a [ref:] marker", asserting calls === 0) is what pins the toVerifierInput trace-field wiring — without it, a dropped knowledgeGraphToolsCalled would make the block-case pass for the wrong reason.

Two findings

1. On merge, this ships as a permanent no-op — and reports green while doing it.

gh secret list --repo byte5ai/omadia returns only the six Apple signing secrets. ANTHROPIC_API_KEY does not exist. So after merge the workflow fires on every main push, the guard sets run=false, every subsequent step (including checkout) is skipped, and the job reports success. The ::notice:: is the only trace, and notices on a green main-push workflow are effectively invisible.

The skip-clean design is right for forks. The problem is that on the primary repo it is indistinguishable from a passing eval, so #129 closes while the regression signal it exists to provide stays at zero — with nothing that will ever go red to remind anyone. Worth either configuring the secret before merge, or making the unconfigured case visibly distinguishable from a real pass (e.g. a step-summary line, or a scheduled-run-only warning annotation).

2. The drift canary can itself drift.

goldenSet.eval.ts pins DEFAULT_MODEL = 'claude-haiku-4-5-20251001'. That currently matches middleware/src/config.ts:445 exactly — but it's a duplicated literal, not an import, and the workflow sets no VERIFIER_MODEL. If someone bumps the verifier model in config.ts, the eval keeps testing the old pin and stays green while production moves. That is precisely weakness #13 (version drift), which is what #129 was filed to close. Deriving the default from config, or asserting the two agree, would close the loop.

Neither is a merge blocker — the harness is well built and strictly better than no eval — but #1 determines whether merging actually delivers the issue's value.

Minor

  • it('reports zero tokens when the stub reports zero usage') asserts 0 === 0; mutating the counter to a constant would keep it green. Cost reporting only, so low impact.
  • The corpus is 13 entries, at the low end of the issue's "10? 50?" — fine for v1, and goldenRunner.test.ts ratchets it with an entries.length >= 12 floor.
  • The weekly cron will be auto-disabled by GitHub after 60 days of repo inactivity. Not a concern at this repo's cadence.

Issue #129's two acceptance hints both check out: the job fails on a regressed verdict class, and the README has an "Adding a corpus entry" procedure.

@Weegy

Weegy commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Addendum — two more, after a second adversarial pass

3. hasRegression([]) is false — a zero-entry corpus is a vacuous pass.

results.some(r => !r.pass) returns false for an empty array, so if loadCorpus() ever yields nothing, the eval prints a 0/0 passed summary and exits 0. Green.

Two things keep this from being urgent, and I want to be accurate about both rather than overstate the finding:

  • loadCorpus() resolves via new URL('./corpus/', import.meta.url), so it is module-relative, not cwd-relative — the usual "CI runs from a different directory" version of this bug cannot happen here.
  • goldenRunner.test.ts asserts entries.length >= 12 against the same directory, in the key-free suite that runs on every PR. Deleting or emptying corpus files goes red there first.

So the realistic regression path is already covered. What remains uncovered is the eval binary itself asserting nothing about how much work it did. A one-line if (entries.length === 0) throw (or a floor constant matching the suite's) would make the eval self-guarding rather than relying on a sibling test.

For completeness on the related error path: API failures are not silently skipped — a throw from pipeline.verify propagates through runCorpus to main().catch, which logs and process.exit(1). That path is correct as written.

4. The eval arms itself by side effect.

The gate is the existence of ANTHROPIC_API_KEY. Nobody ever performs an action that means "turn the golden eval on." The day that secret is added for any unrelated reason — a different workflow, a one-off script, an org-wide rollout — this weekly cron plus every-main-push job silently starts spending tokens, with no one having decided to enable it.

A separate repo variable as the arming switch (vars.GOLDEN_EVAL_ENABLED), ANDed with key presence, decouples "we have a key" from "we want to pay for this eval" and makes enabling it an auditable action in the repo settings log.

On finding #2 (the duplicated model literal): worth converting into a key-free unit test rather than a review note — assert that the eval's DEFAULT_MODEL equals the VERIFIER_MODEL default from config.ts. It runs in npm test, costs nothing, needs no secret, and closes the drift path permanently instead of relying on someone remembering.

Not concerns, checked and cleared: the workflow already has timeout-minutes: 20, a concurrency group with a deliberate cancel-in-progress: false, and permissions: contents: read. It correctly uses a step-level guard emitting an output rather than secrets in a job-level if: (which would not evaluate as written). And since the trigger set excludes pull_request, fork PRs never attempt it — worth keeping that way, since this PR is itself from a fork and a pull_request trigger would guard-out to green on exactly the external contributions where regressions arrive.

One operational note: cron-failure notifications go to whoever last modified the workflow file. Post-merge that is @sneumannb5, not the maintainer team — worth a deliberate owner once the key is configured, otherwise a red canary reaches nobody.

@Weegy
Weegy merged commit 75eb5d7 into byte5ai:main Aug 11, 2026
9 checks passed
Weegy added a commit that referenced this pull request Aug 21, 2026
…lders audit row (#820)

Two findings from the same review wave, one branch because both are small
and sharp.

#764 — CI never ran the workspace packages' own test suites. `npm run test`
globs only `middleware/test/**`; canvas-core (vitest), conductor-core
(vitest) and plugin-api (node:test) ran nowhere. Not hypothetical twice
over: #759 broke two conductor-core tests while every required check stayed
green, and #725's 15 canvas-core validator tests were only ever run by hand
during review. New CI step runs all three. `@omadia/plugin-ui-helpers` is
deliberately absent: it declares a test script but contains zero test files,
and papering over that with --passWithNoTests would create a permanently
green no-op — the exact failure family this repo keeps finding (#640, #752).
If it gains tests, add it to the step.

#775 — the `conductor.role_holders_change` audit entry never landed: the
index.ts closure passed the session sub (an EMAIL under local auth) as
`actor.id`, and `admin_audit.actor_id` is a uuid column, so every insert
threw. Loud in the log, empty in the audit trail — the entire point of #759.

Fix: the conductor entry additionally threads the session's
`omadia_user_id` (a real uuid), and the mapping lives in an exported
`roleChangeAuditEntry` (adminAuditLog.ts): uuid to `actor_id` only when the
session carries one, the sub always to the free-text `actor_email` — the
same treatment the adminUsers routes give it, and for the `'operator'`
fallback the only place the actor survives at all.

Tests: 3 mapper units; the existing route test now proves the uuid threading
end-to-end (harness session carries omadia_user_id, deep-equal asserts it);
and a pg-gated suite runs the mapper output through the real
`AdminAuditLog.record` against the REAL migration DDL (read from the
migration file, not copied — a hand-duplicated schema could drift and
green-light what production rejects). Its second case pins the regression
permanently: the OLD mapping must keep failing on the real column with
`invalid input syntax for type uuid`.

Mutation checks: removing the uuid threading turns the route test red;
reverting the closure to the old inline mapping reproduces exactly the
insert the pg test asserts the database refuses. Verified against an
ephemeral postgres:16-alpine (2/2).

Full suite: 7048 tests, 1 pre-existing-shape failure fixed by extending the
existing expectation (the new field), then green; typecheck:test ratchet
held with no regressions; lint clean.
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.

feat(ci): golden-set regression eval for LLM output behaviour

2 participants