Make starter money checks exact - #3162
Merged
Merged
Conversation
The first public rubric-judge smoke run exposed two gaps: a substring assertion accepted 33.2366 as 33.23, and the starter calculator could not perform the rounding operation selected by the agent. Require bounded positive dollar amounts, teach cent-exact allocation, and support safe decimal rounding before publishing the next patch. Constraint: The starter eval must remain fast and understandable while combining deterministic and model-based grading. Rejected: Remove the no-failed-tools gate | would hide unsupported calculator calls. Rejected: Rely only on the LLM judge | would make exact monetary correctness nondeterministic. Confidence: high Scope-risk: narrow Directive: Keep deterministic currency gates alongside the rubric judge; do not replace them with substring checks. Tested: Template test; manifests; fmt; lint; typecheck; live 3-repetition eval with 3/3 passing all gates. Not-tested: Full suite has two unrelated baseline failures in docs coverage and a missing generated npm artifact.
kwakayama
previously approved these changes
Jul 29, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens the ai-agent starter template’s deterministic evaluation to require cent-exact currency tokens (avoiding substring false positives like 33.23 matching 33.2366...), extends the starter calculator tool with a bounded round operation, and bumps the runtime patch version to 0.1.1174.
Changes:
- Bump runtime version from
0.1.1173to0.1.1174(deno.json,src/utils/version-constant.ts). - Update the
ai-agentstarter: stronger system instruction for currency splits, eval gates switch fromanswer.containstoanswer.regexfor exact dollar tokens, and addroundto the calculator tool (including template regression assertions). - Update template regression tests to lock in the new eval and calculator template behavior.
Verification (reported in PR description):
deno test -A cli/templates/index.test.ts: 1 passed, 15 steps- canonical pre-push suite: 2716 passed, 22089 steps, 0 failed, 5 ignored
- additional eval repetitions and broader tests reported by author (not re-run in this review)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/version-constant.ts | Patch version bump to 0.1.1174. |
| deno.json | Patch version bump to 0.1.1174. |
| cli/templates/manifest.json | Updates scaffolded ai-agent template payload (assistant system prompt, eval regex gates, calculator round). |
| cli/templates/index.test.ts | Adds regression assertions for the new template strings and regex behavior. |
| cli/templates/files/ai-agent/tools/calculator.ts | Adds round operation with bounded precision. |
| cli/templates/files/ai-agent/evals/assistant.eval.ts | Switches deterministic money checks from contains to regex with token boundaries. |
| cli/templates/files/ai-agent/agents/assistant.ts | Adds explicit instruction to make rounded currency shares sum exactly to the total. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kojiwakayama
disabled auto-merge
July 29, 2026 06:41
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7683777669
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Binary floating-point representation can place half-cent values just below their intended decimal boundary. Apply a sign-aware, magnitude-scaled one-ULP offset before decimal formatting so the starter handles positive and negative currency values symmetrically. Constraint: Keep the starter dependency-free and preserve the existing 0-100 precision contract. Rejected: Raw toFixed rounding | produces 1.00 for 1.005 at two decimals. Confidence: high Scope-risk: narrow Directive: Keep the source template and generated manifest synchronized. Tested: Template suite including positive and negative half-cent execution; manifest check; full lint; full typecheck.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
cli/templates/index.test.ts:182
- The template regression test asserts the new strict money regexes for $15.21, $33.24, and $33.23, but it no longer asserts the $99.71 total check. Since this PR’s goal is cent-exact gating, missing the total assertion weakens the guard against regressions in
evals/assistant.eval.ts. Add a matching assertion for the $99.71 regex pattern.
assertEquals(
assistantEval.includes(
"metrics.answer.regex({ pattern: String.raw`(?<![-\\d.])\\$15\\.21(?![\\d.])` }).gate()",
),
true,
);
Markdown-escaped dollar signs allowed the regex engine to restart at the dollar token and skip a preceding minus. Match the optional escape as part of the amount token and reject backslashes at the lookbehind boundary. Constraint: Continue accepting both plain and Markdown-escaped positive dollar amounts. Rejected: Disallow escaped dollar signs | valid model Markdown output uses them. Confidence: high Scope-risk: narrow Directive: Apply the same token boundary to every exact-money gate in this starter. Tested: Regression for escaped negative amount; template suite; manifest check; focused lint; full typecheck.
kojiwakayama
enabled auto-merge
July 29, 2026 06:52
kwakayama
approved these changes
Jul 29, 2026
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
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.
Description
The public
0.1.1173quickstart smoke test exposed a false-positive in the generated AI Agent eval:answer.contains(\"33.23\")accepted the incorrect intermediate value33.2366.... The LLM rubric judge rejected the answer, but the deterministic amount gates should have rejected it too.This patch makes the starter cent-exact end to end:
roundoperation to the calculator tool0.1.1174Related Issue(s)
Follow-up to #3161.
Type of Change
Verification
deno test -A cli/templates/index.test.ts: 1 passed, 15 stepsThe repository-wide
deno task testalso ran with 3169 passing tests and two unrelated generated-artifact baseline failures (release-assetsdocs coverage and an absent local npm build output).Checklist