Skip to content

eval: add first TradeEvaluation LLM judge slice - #705

Merged
atomchung merged 10 commits into
mainfrom
codex/issue-590-trade-answer-judge
Aug 1, 2026
Merged

eval: add first TradeEvaluation LLM judge slice#705
atomchung merged 10 commits into
mainfrom
codex/issue-590-trade-answer-judge

Conversation

@atomchung

@atomchung atomchung commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Refs #590. Refs #579. Refs #676. Related but independent: #610.

User before / after

Before, maintainers could replay deterministic TradeEvaluation facts and question-level judge axes, but could not collect auditable answer-level evidence for a final answer that buried the decision, contradicted itself, merely restated fields, or padded caveats. A frozen witness bank alone also could not detect a regression in a newly captured product/agent answer.

After, maintainers have two explicit opt-in paths over one fictional TradeEvaluation:

  • fixture_witness calibrates the judge against orthogonal committed mutations.
  • candidate_output --answer-file ... grades the exact complete answer captured for the same frozen evaluation without editing the witness bank.

Both paths run production provenance, challenge-delivery fidelity, and shared number/date provenance checks before any model call. The semantic judge then emits repeated per-axis evidence into a durable local receipt. The result remains advisory and uncalibrated until the owner ratifies the labels.

Two independent lanes

Neither lane consumes the other's verdict, and no #610/private trade material enters this public bank or the external judge route.

What changed

  • Reuse evals/judge_episodes.py for backend resolution, blind single-answer prompting, repeated votes, fail-closed parsing, and ambiguity behavior; preserve explicit TR_JUDGE_AGY_PATH override with portable PATH discovery otherwise.
  • Add four bounded axes: decision_focus, internal_consistency, decision_synthesis, and caveat_discipline.
  • Add one fictional same-challenge bank with an isolated failing witness for every axis, two passing witnesses, and one deterministic rejection witness.
  • Add schema-v3 --answer-file input for a byte-for-answer-equivalent normal two-paragraph-plus-resolution candidate. Ordered Unicode-character segments partition the complete presented_text: each production-validated case claim is reproduced exactly once; limitations may reference only existing challenge obligations; connective prose is declared agent judgment; the single final resolution declares and visibly names open/declined/modified; separators are whitespace only.
  • Reject gaps, overlaps, out-of-range/bool offsets, unknown segment fields/kinds, repeated/omitted/mismatched claims, fabricated limitation refs, blank substantive segments, unbound trailing text, bad resolution structure/markers, candidate/run-kind mismatches, and artifacts that do not reproduce every surface the model will see. These all fail before a model call.
  • Keep the semantic boundary explicit: mechanical markers do not prove arbitrary limitation/connective/resolution prose honestly fulfils its label. The relevant four axes and owner review retain that responsibility; resolution meaning is outside this four-axis judge.
  • Run the production answer_provenance validator, shared challenge-delivery fidelity, and number/date provenance against the complete answer. Negative engine deltas may be displayed as positive magnitudes with directional prose; direction remains structured-case/internal-consistency work.
  • Show the judge only the question, frozen evaluation, production challenge, and one answer. It never sees witness IDs, sibling answers, expected labels, case refs, titles, or maintainer notes.
  • Require every verdict to carry a non-empty reason. Missing axes, partial/unreadable samples, empty reasons, ties, and backend errors fail closed and cannot count as agreement.
  • Persist schema-v3 receipts under the protected coach root. Receipts retain the candidate artifact plus content-addressed canonical call specs actually used by the backend. A locked preflight rejects torn, malformed, or already-unwritable real history before model calls; append repeats full UTF-8/JSON/torn validation under lock, rolls back failed writes/file-syncs, and happens before stdout output. A later final-write failure is explicitly non-evidence.
  • Keep receipt files inside coach.py status/export/reset governance. Model calls remain opt-in and outside CI.

Synthetic semantic evidence

The earlier live calibration run used the unchanged witness bank, prompt, and four-axis contract: agy / gemini-3.1-pro-high, three samples per eligible answer.

  • Six eligible witnesses; one production-gate rejection made no model call.
  • Every axis matched every declared witness label: 6 agreement / 0 disagreement / 0 ambiguous for each axis.
  • Calibration remains declared_by: agent; this proves reproduction of the bank, not owner accuracy or product acceptance.

The final corrections changed candidate ingestion and deterministic receipt interlocks, not those witness labels or semantic prompts, so no additional billable run was used as a merge gate.

Verification

  • python3.12 tests/run_all.py: 53/53 suites passed after integrating origin/main@cc67f93.
  • TradeEvaluation judge interlocks: 28/28 passed.
  • Existing episode checker probes: 91/91 passed.
  • Cross-client interaction trajectory: 142/142 passed.
  • python3.12 evals/judge_trade_answers.py --plan: 18 model calls planned, with six eligible witnesses and one deterministic skip.
  • Two independent final adversarial reviews reported no blocker/major after replaying the complete-answer, exact-artifact, run-kind, negative-delta, and torn-history counterexamples.

Python 3.11 and 3.12 remain covered by the normal PR CI matrix. A billable/nondeterministic semantic call is intentionally not part of CI.

Review / calibration decision

No product or merge-blocking choice is delegated to the LLM. Owner label ratification remains an optional next calibration step and is separate from #610's private human walk.

Copy link
Copy Markdown
Owner Author

Review finding — park before merge: the owning contract has not authorized implementation

This PR is bounded and maintainer-only, but it is currently sequenced ahead of its own evidence gate.

Disposition: do not merge this PR in the current state. Park it or convert it to draft. The smallest valid resume condition is:

  1. run [QA·dogfood] Judgment-quality dogfood — does a real consider answer now carry a supported insight? #610 on the frozen SHA;
  2. convert an actual de-identified answer-quality miss into the first synthetic witness;
  3. record the owner's expected per-axis verdicts on [eval·research] Judge TradeEvaluation answer quality without rewarding verbosity or copying a reference answer #590;
  4. then rebase this implementation and review whether the thin adapter is still the smallest needed cut.

This is a sequencing/ownership block, not a finding that the code architecture is necessarily wrong. I have not treated CI or the synthetic bank as acceptance evidence because the owning issue deliberately requires observed product evidence first.

@atomchung

Copy link
Copy Markdown
Owner Author

Owner sequencing update — human and LLM lanes proceed separately

The former parking reason on this PR is superseded by the owner's explicit direction in this work session:

This changes sequencing only; it does not turn the synthetic judge into product acceptance or import any private #610 material.

@atomchung atomchung left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review result: changes required before merge

CI is green and several boundaries are implemented carefully: deterministic eligibility precedes model calls, the judge is blind to sibling/reference answers, per-axis ambiguity fails closed, calibration remains explicit, and receipts are covered by local data controls. I do not see a privacy or canonical-state defect in this diff.

Two findings block merging this as the first automated FOMO QA slice.

[P1] The bank grades committed prose, not the current product/agent output

TA-001 stores every answer's prose directly, and run_judge() sends that frozen prose to the judge. Nothing in this PR runs the current consider route through a supported host/model, captures the answer actually generated under the current SKILL.md / docs/output-voice.md / route prompt, or substitutes that output into the fixture.

Consequence: a later prompt/model regression can make the real product bury the lead, contradict itself, or become disclaimer-heavy while this suite remains byte-for-byte green, because it continues judging the same hand-authored passing witnesses. This is useful rubric calibration, but it is not yet automated workflow QA or a prompt/model regression harness.

Smallest viable correction:

  1. Keep the frozen witnesses as judge mutation/calibration assets.
  2. Add a candidate-generation input path that accepts or produces the current answer for the same frozen TradeEvaluation payload, then runs deterministic delivery/provenance checks and the semantic judge on that candidate.
  3. At minimum, support --answer-file (or a structured run artifact) and clearly report fixture_witness versus candidate_output. Host execution can remain a later slice, but frozen-witness grading must not be presented as product regression coverage.
  4. Add a regression proving that changing the candidate answer changes the result without editing TA-001.

Without this, the harness proves the judge can reproduce labels selected by the author; it does not test whether FOMO Kernel currently produces a good answer.

[P2] Shared agy discovery regresses from PATH lookup to one machine-specific location

evals/judge_episodes.py previously used shutil.which("agy"), so any valid PATH installation worked. This PR changes the default to ~/.local/bin/agy and considers agy unavailable everywhere else unless the maintainer discovers and sets TR_JUDGE_AGY_PATH.

That is a backward-compatibility regression in the pre-existing question-episode judge, outside the new TradeEvaluation adapter. Homebrew, pipx, uv-tool, and other PATH installations can stop resolving.

Smallest correction: preserve TR_JUDGE_AGY_PATH as an explicit override, otherwise fall back to shutil.which("agy"); use the resolved absolute path for subprocess execution. Add tests for override and PATH-discovered cases.

Non-blocking assessment

  • A dedicated answer-level adapter is defensible, but it is already becoming a second fixture/loader/coverage/report architecture beside evals/episodes. Keep the first bank bounded and do not expand routes before the candidate-output path proves real regression value.
  • The durable receipt design is stronger than required for synthetic-only calibration, but reasonable because future candidate outputs may contain private decision text and it is correctly governed by data-status/export/reset.
  • Owner ratification must remain separate from #610 owner-live acceptance, as the PR states.

Please address P1 and P2, then rerun the full suite and the opt-in --plan path. A real billable judge pass is not required for these code corrections.

@atomchung

Copy link
Copy Markdown
Owner Author

Review feedback addressed in 5b88f02

Both requested changes are implemented.

P1 — current candidate output is now a separate lane

  • fixture_witness remains the committed calibration/mutation bank.
  • --answer-file creates an explicit candidate_output run without editing TA-001.
  • Candidate schema v2 binds the exact surface to every production-validated agent_case claim exactly once. An extra unlabelled factual sentence fails in both the loader and direct eligibility path before any model call.
  • The exact candidate artifact, run kind, source-fixture digest, candidate digest, deterministic delivery evidence, and content-addressed model call are preserved in the local receipt.
  • Regression tests prove a validated candidate change changes the result/digest without changing TA-001, while a wrong number or unlabelled claim makes zero model calls.

Host/model generation remains intentionally outside this first slice; the receipt does not claim this script generated the captured answer.

P2 — portable agy discovery restored

  • TR_JUDGE_AGY_PATH remains an authoritative explicit override.
  • Otherwise discovery uses shutil.which("agy") and executes the resolved absolute path.
  • Override, PATH discovery, missing executable, launch OSError, timeout, and retry paths are covered.

Additional adversarial findings closed

  • Deterministic eligibility now also uses the shared production challenge-delivery and number/date provenance checks on the exact presented surface.
  • Partial/unreadable samples are ambiguous, never agreement; per-axis agreement/disagreement/ambiguity counts and rates are in receipts/history.
  • Receipts retain the exact call spec actually executed, including prompt/messages and transport options; candidate artifacts remain reconstructable after the temporary input file is deleted.
  • Output is emitted only after receipt append/fsync, so a closed pipe cannot erase completed-call evidence.
  • JSONL appends are locked, reject torn tails, pre-sync the directory, roll back partial/file-fsync failures to the exact prior offset, and safely round-trip hostile Unicode.
  • The synthetic caveats were reattached to the claims they narrow; the final independent judge run showed 6 agreement / 0 disagreement / 0 ambiguous on every axis.

Verification

  • Full suite: 53/53
  • TradeEvaluation interlocks: 23/23
  • Episode checker probes: 91/91
  • Interaction trajectory: 142/142
  • Final live run: agy / gemini-3.1-pro-high, 3 samples × 6 eligible witnesses; all four axes 6/6 agreement
  • Two final independent adversarial re-reviews: no blocker/major

#610 remains a separate private human lane and is not accepted by this result. Owner ratification of the synthetic labels remains optional calibration, not an inferred LLM decision.

@atomchung atomchung left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review result: one remaining blocker

The previous P2 is correctly fixed: an explicit TR_JUDGE_AGY_PATH remains authoritative, otherwise the shared judge falls back to shutil.which("agy"), resolves an absolute executable path, and CI is green on the latest head.

The previous P1 is only partially fixed.

[P1] candidate_output still cannot faithfully ingest a normal product answer

--answer-file now exists, but _claim_bound_presentation() requires:

  • every visible byte to be one of agent_case.for[] or agent_case.against[] claims;
  • every claim to appear exactly once;
  • presented_text to equal the newline join of those claims exactly.

That is not the normal TradeEvaluation response contract. A real answer also contains material that is neither a case-for nor a case-against claim, for example:

  • the attached basis limitation;
  • a grouped unchecked boundary;
  • the user-owned resolution sentence (keep open / modify / decline);
  • connective synthesis between grounded claims.

To feed such an answer into this lane, the capture step must either delete that text or misclassify it as a for/against claim. In both cases the artifact is no longer the exact answer the user saw. The current tests therefore prove that changing a specially authored claim-only candidate changes the judge result; they do not yet prove that an ordinary current FOMO Kernel answer can be captured and replayed without rewriting it.

This also weakens the four selected axes: caveat_discipline is being judged only after caveats have been forced into the claim ontology, and the final resolution surface cannot be represented honestly at all.

Smallest correction

Keep production validation on the structured agent_case, but bind it to the visible answer through spans/segments rather than making the whole answer equal the case claims. For example:

{
  "presented_text": "...exact full answer...",
  "segments": [
    {"kind":"claim_ref","side":"against","index":0,"start":0,"end":42},
    {"kind":"limitation","start":43,"end":91},
    {"kind":"resolution","start":92,"end":130}
  ]
}

Then require every structured claim exactly once, reject overlapping/out-of-range spans, run number/date provenance and challenge coverage over the complete presented_text, and constrain non-claim segments to the existing limitation/resolution obligations. Another equivalent design is fine; the key acceptance test is:

Take an ordinary two-paragraph-plus-resolution TradeEvaluation answer produced under the current route contract, store it byte-identically, and pass it through --answer-file without rewriting any sentence into a special claim-only form.

Until that works, describe this lane as a claim-only candidate witness, not a current product-output regression path. Because the PR body currently claims it grades an “exact newly captured answer,” I still do not recommend merge in the current state.

Remaining assessment

  • CI on 5b88f02 is green.
  • Receipt durability, ambiguity accounting, calibration disclosure, privacy controls, and PATH compatibility look sound.
  • The implementation has grown to ~2.7k added lines for one first-cut evaluator. Do not add another persistence or backend abstraction while fixing this; narrow the candidate contract rather than expanding the framework further.

@atomchung

Copy link
Copy Markdown
Owner Author

Final re-review update — remaining P1 and adversarial follow-ups closed

Latest head: 9fc49f4, including origin/main@cc67f93.

The claim-only candidate restriction is replaced by schema-v3 full-answer segmentation. An ordinary two-paragraph-plus-resolution TradeEvaluation answer now round-trips character-for-character. The complete surface is partitioned into exact claim_ref, bounded limitation, declared connective, final resolution, and whitespace separator segments; every structured claim appears exactly once and no text may remain unbound.

The final independent reviews also found and verified fixes for four direct-call/evidence counterexamples:

  • the candidate artifact now binds _presented_text() across every surface the model sees, not prose alone;
  • candidate_output cannot run under fixture_witness receipt semantics;
  • a negative engine delta may be surfaced as its positive display magnitude with directional prose;
  • torn or malformed real receipt history is rejected before the first model call, then revalidated under the final append lock.

Resolution option markers are mechanically checked, while the docs explicitly avoid claiming those markers prove arbitrary prose honestly offers the choices. That remaining semantic boundary stays with the relevant judge axes and owner review.

Verification on the integrated head:

  • full local suite: 53/53;
  • TradeEvaluation interlocks: 28/28;
  • episode checker probes: 91/91;
  • interaction trajectory: 142/142;
  • opt-in plan: six eligible witnesses × three runs = 18 planned calls, plus one deterministic skip;
  • two independent final reviews: no blocker/major.

#610 remains the separate private human lane and is not accepted by this result.

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