chore(templates): make the starter eval readable - #3426
Merged
Conversation
The ai-agent starter eval is the first eval most people ever read. It opened
with four hand-rolled lookaround regexes:
metrics.answer.regex({
pattern: String.raw`(?<![-\d.\\])\\?\$33\.23(?!\d|\.\d)`,
}).gate(),
Those are exact, and they are exact for good reasons earned the hard way:
#3162 replaced `contains({ text: "33.23" })` because it passed on 33.2366, and
a follow-up commit added the `\\?` and the backslash in the lookbehind because
Markdown-escaped output let the engine restart at the dollar token and skip a
preceding minus.
But a starter file teaches by example, and this one taught that asserting an
agent said "$33.23" takes a negative lookbehind. Nobody reading it learns what
the eval checks; they learn to copy a regex they cannot modify.
The exactness requirement moves into the rubric judge, in prose:
"Every amount must be exact to the cent: $33.2366 and $133.23 are wrong."
That is the same constraint, stated so a reader can check it and change it.
This is a deliberate trade. #3162's directive was to keep deterministic
currency gates alongside the rubric judge, and this branch does not: monetary
correctness is now graded by a model, so it costs an LLM call and can vary run
to run. The gate threshold stays at 0.8 and the tool gates
(`calledTool("calculator")`, `noFailedTools()`) stay deterministic, so a run
that skips the calculator or errors a tool still fails without the judge.
The regression worth guarding is the file drifting back toward unreadable, so
the template test asserts the eval carries no `metrics.answer.regex` and no
`String.raw`, and -- because nothing else enforces exactness now -- that the
rubric still names all four amounts, still says "exact", and still shows the
near-misses it has to reject.
Tested: template + eval suites (23 files, 217 steps); fmt; lint; typecheck;
manifest check. Mutation-checked the new guard by dropping the exactness
sentence from the rubric and confirming the test fails.
Not-tested: no live eval run against a model; the rubric judge's ability to
reject $33.2366 in practice is asserted in prose, not measured.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe assistant eval now uses a rubric judge for exact tip, total, and per-person split values. Documentation and the manifest describe the expected results. Template tests verify rubric configuration and reject regex-based validation. ChangesAssistant eval rubric migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The
ai-agentstarter eval is the first eval most Veryfront users ever read. It opened with four hand-rolled lookaround regexes:This replaces them with the rubric judge that was already in the file.
Why the regexes existed
They are exact, and exact for reasons earned the hard way:
contains({ text: "33.23" })because it passed on33.2366.\\?and the backslash in the lookbehind because Markdown-escaped output (\$15.21) let the engine restart at the dollar token and skip a preceding minus.Both were real bugs. Neither is in dispute.
Why they still go
A starter file teaches by example. This one taught that asserting an agent said "$33.23" takes a negative lookbehind. A reader does not learn what the eval checks — they learn to copy a regex they cannot modify. That is the wrong first lesson.
The exactness requirement moves into the rubric, in prose:
Same constraint, stated so a reader can check it and change it.
The trade — please read before approving
#3162 carried an explicit directive: "Keep deterministic currency gates alongside the rubric judge; do not replace them with substring checks." This branch knowingly departs from it. Monetary correctness is now graded by a model, which means it costs an LLM call and can vary between runs.
What holds the line instead:
0.8.calledTool("calculator")andnoFailedTools()stay deterministic, so a run that skips the calculator or errors a tool still fails without involving the judge.$33.2366,$133.23) explicitly rather than leaving "exact" to interpretation.This was a deliberate call by the requester after being shown the regression risk. If reviewers would rather keep deterministic money gates, the alternative I'd recommend is a first-class
metrics.answer.containsAmount({ amount: "$33.23" })inveryfront/eval— the boundary logic tested once in the SDK instead of copy-pasted into every user's project. Happy to switch to that.Test changes
accepts sentence punctuation without accepting longer monetary valuestested the four regex metrics directly and goes with them. In its place,grades the starter's money answer with a rubric a reader can followguards both halves of the change:metrics.answer.regexand noString.raw— the readability property.The existing
metrics.answer.contains(assertion stays, so the #3162 substring bug cannot come back either.Verification
deno task fmt:check— passdeno task lint— passdeno task typecheck— passdeno task generate:manifests:check— pass (manifest.jsonregenerated)Not tested: no live eval run against a real model. The judge's ability to actually reject
$33.2366is asserted in prose, not measured. That is the main thing a reviewer should push back on if they are uncomfortable.Summary by CodeRabbit
Enhancements
Tests