Repository navigation
fix(answer-commit): infer column scale and accept finite decimals - #1516
Merged
anandgupta42 merged 7 commits intoOct 16, 2026
Merged
Conversation
- Infer the maximum decimal scale per numeric column (by position) when `rows.precision` is omitted, so `5` and `4.18` render as `5.00` and `4.18` in markdown, csv, tsv and lines output. - Explicit `rows.precision` still overrides inference. - Drop the 15-significant-digit limit on numeric values; unsafe integers are still rejected and NaN/Infinity are still rejected by Zod. - Update the tool and schema descriptions and add regression tests. Part of #1481 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: c2218b5a-683a-4c7d-bf39-3ce2d2af1ee2) |
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous Review Summaries (6 snapshots, latest commit 5b8513d)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 5b8513d)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Previous review (commit 1edba4f)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 9c55371)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit f581dbb)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 477f834)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 54efbd1)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Reviewed by gemini-4-argon · Input: 106.4K · Output: 917 · Cached: 333.9K Review guidance: REVIEW.md from base branch |
- Cells with more than 6 decimals that do not survive 12-significant-digit rounding no longer raise the inferred column scale. - Noise cells render at the inferred scale, or in their own short form when no other cell sets a scale. - Add regression tests for a noise cell among clean cells, a noise-only column, and genuine high-scale values. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Addressed the review finding "Inferred column scale spreads float noise": cells with more than 6 decimals that do not survive 12-significant-digit rounding (e.g. The two low findings are left as is: field/row scientific-notation divergence is existing scalar behavior outside this PR, and the non-finite rejection test is cheap extra coverage. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 606147b3-b936-4be1-b4e5-5df925b59575) |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
![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: 477f834eff
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
… rule - Treat a cell as float noise only when it is within 1e-7 relative error of its 6-decimal rounding, so float32 and binary artifacts are ignored but genuine long decimals set the column scale. - Add a regression test for genuine long decimals beside clean cells. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: d75ae27c-83f7-4f5c-a3bc-c9ba545fa2aa) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f581dbb1be
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… artifacts Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 3f974070-6f4d-417c-9a89-51138771c9cf) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c55371c88
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… rounding Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 620d067c-a4c9-4077-9cde-31a27f68368d) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1edba4fc5d
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- State in the tool description and analyst prompt that each item the request names (an entity and a related item or count) must be its own field, since basis and prose are hidden under answer-only output. - Add a test pinning the guidance and the answer-only rendering of a complete field set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 4661319a-c5b3-442e-887c-564a908df694) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b8513daa9
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- A recognized float artifact now contributes the scale of the shortest value it is an artifact of, so a column mixing it with coarser cells keeps the intended digits. - A column containing only noise cells keeps its values as given. - Add a regression test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 0166c078-40b7-49b5-820f-d21e84ebfda4) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccc03c9e11
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
anandgupta42
added a commit
that referenced
this pull request
Oct 16, 2026
…ally (#1449) * feat(analyst): `answer_commit` renders the final answer deterministically The analyst's final message was free prose shaped by a prompt paragraph, so the same computed values could be presented differently from run to run. `answer_commit` is an ordinary tool, available only to the analyst agent, that takes the single chosen value(s) (`fields`), an N-row table (`rows`) or `unresolved` with a reason, plus an optional two-sentence basis and `answer_only` flag, validates the payload structurally (one value per field, no empty values, rectangular rows, precision applied, at most two basis sentences, strict-schema fillers treated as absent) and returns the exact final text to reply with. An advisory `readings` ledger (clause, reading, result summary, decision) is kept in tool metadata and never rendered. - Session mechanics, message rendering, loop termination and tracing are unchanged; the tool only returns text. - Registered on the analyst-only gated candidate list, so builder, plan, data-explorer and custom agents cannot see it and `tool_lookup` does not list it; `ALTIMATE_ANSWER_COMMIT=0` removes it. - The analyst prompt's final paragraph now asks to finish by calling `answer_commit` and replying with exactly the returned text, with the previous prose contract kept as the fallback when the tool is unavailable. - Not covered: automatic removal in user JSON-schema mode (no hook exists where `StructuredOutput` is injected); documented in the tool description. Tests: 48 renderer/validation/tool cases, 4 registration and scoping cases, prompt-contract assertion updated. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): `answer_commit` description, filler tolerance, escaping and sentence counting Review follow-ups: - The tool description says when to call it (last tool call, once the answer is decided), that the returned text is the verbatim final reply, what `answer_only` means, and that a user output schema takes precedence. - The prompt's unavailable-tool fallback restores the answer-only rule. - `null` and whitespace-only values for optional fields are treated as absent; whitespace-only `value`/`name` is the existing empty-value error. - Table cells escape `|` and a preceding backslash; newlines become spaces. - The two-sentence basis cap ignores common abbreviations and decimals. - Tests cover the above, a custom agent named `analyst` (receives the tool by design) and a same-id plugin tool. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): `answer_commit` returns only the answer text; sentence counting and precedence test tightened Independent-review follow-ups: - The tool output is exactly the rendered answer; the "reply with the returned text" instruction lives in the tool description, so the prompt and the tool no longer disagree about what to echo. - The basis sentence counter treats a boundary as `.`/`?`/`!` followed by whitespace (including newlines) and any non-space character, still ignoring common abbreviations and decimals; lowercase/newline cases tested. - The plugin-precedence test runs the real tool resolver instead of re-implementing its selection logic. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): `answer_commit` rejects unsafe integers, echoes at the step limit, uses native Effect Review-bot findings of the highest severity tier plus their siblings: - Numeric field values and table cells above `Number.MAX_SAFE_INTEGER` are rejected (JSON parsing would have rounded them); exact strings are accepted instead. - At the final allotted generation step, an analyst turn whose previous step completed `answer_commit` is asked to reply with exactly the returned text, with tools disabled and a user output schema still taking precedence, instead of being asked for a summary that would discard the committed answer. - The tool exposes the namespace projection the registry consumes, runs as a native Effect definition with native permission handling, and its tests use scoped Effect fixtures. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(tools): valid draft 2020-12 schema for `answer_commit` fillers; omit root `$schema` for Anthropic SDKs A live probe against Bedrock Claude rejected the analyst tool list: `tools.45.custom.input_schema: JSON schema is invalid. It must match JSON Schema draft 2020-12`. The `answer_commit` optional-filler helper used `z.tuple([])`, which Zod emits as `"prefixItems": []`, invalid under 2020-12 (`prefixItems` needs at least one item). OpenAI accepted the same schema, so only Anthropic-family providers were affected. - Replace the empty tuple with `z.array(z.never()).length(0)`, which still accepts exactly an empty array and keeps union error selection. - Omit the root `$schema` declaration in the provider transform for the Anthropic, Bedrock and Vertex Anthropic SDKs. - Add `tool-schema-anthropic.test.ts`: every registered tool schema for the analyst and builder agents is validated against the draft 2020-12 meta-schema (ajv, dev dependency) plus Anthropic's restrictions, so an invalid tool schema fails in CI instead of at the provider. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): reserve the `answer_commit` id against external tools; keep the committed text exact through compaction Two review findings of the highest severity tier, each with red-first tests: - The native `answer_commit` id is reserved: an MCP, plugin or file tool with the same name (including Windows case-equivalent spellings, and with the toggle off) cannot take the analyst's finalizer permission. - The committed answer text survives automatic compaction and provider retries unchanged; continuation ids are tracked so a newer request is not overwritten by an older echo. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): restore the top-N tie contract and the answer-only fallback for `answer_commit` Two contract regressions raised in review: - The analyst prompt again says a top-N with a tie at the boundary returns exactly N concrete rows with a brief tie note; the tool description and the `rows` schema say the same, with `rows.note` as the place for the note. - The unavailable-tool fallback requires exactly the requested values or table in the requested format, with no label and no basis, for answer-only or specific-format requests; `answer_only` rendering stays label-free. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): `answer_commit` accepts boolean-like strings, requires a basis, and explains attribution Findings from live Claude sessions on Bedrock: - Every call failed validation because the model sent `answer_only` as the string `"true"`. Boolean fields now accept case-insensitive `"true"`/`"false"` and `1`/`0` at runtime; the published schema type stays boolean. - Sessions that dropped the previous "state your basis" sentence bound a policy clause about a related object to the requested object's attribution. `basis` is now required unless `answer_only` is set and must name population, grain and the source object/column that supplies the requested entity or value; the tool description and the analyst finishing paragraph say so and separate policy eligibility from metric and entity attribution. - Validation failures return a one-line error plus a minimal valid example payload so the next call can repair instead of repeating the same input. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): do not duplicate the answer label when the single field is named Answer A single field named Answer, Result or Value (or left blank) renders as the plain answer line instead of repeating the label. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): require a basis on every answer_commit payload The basis (population, grain, source object/column) is the model's own reasoning input; it is required even for answer-only requests, where it is validated but not rendered. Live sessions that skipped it committed an entity from a related object instead of the requested one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): echo after every commit, keep `StructuredOutput` requests tool-free, cap and type-check payloads - Structured-output requests (`format.type === "json_schema"`) no longer register `answer_commit`, never enter echo mode, and keep earlier commit tool parts as masked text so a historical stub cannot re-advertise the tool; the analyst prompt says `StructuredOutput` takes precedence for those requests. - Echo mode now starts right after any completed `answer_commit` for the current request, not only at the step limit or after compaction: the next generation is tool-free and carries the exact echo instruction. A failed commit keeps the tools for repair; a new user message resets finalization. - Rendered-size cap: `MAX_RENDERED_ANSWER_CHARACTERS` (60,000) applies to the complete rendered text of fields, rows and unresolved payloads; the error and the tool description tell the model to aggregate, narrow, or write the full table to a file and commit the summary. - Numeric precision: numbers with more than 15 significant digits or outside the safe-integer range are rejected with instructions to pass the exact value as a string (JSON parsing already changed `1234567890123456.7` to `…456.8`); strings are unchanged. - Regression tests for each: schema-mode tool list and masked history, echo after commit with unbounded steps and with a sibling tool, failed-commit repair, reset on a new user message, cap boundaries (59,999 / 60,000 / over), escaped cells and basis counted, and the parsed precision-loss literals plus safe-number controls. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): send the commit echo instruction as a user turn Bedrock's Converse API rejects a request whose last message has the assistant role ("conversation must end with a user message", HTTP 400), so the tool-free echo step after `answer_commit` failed live and the run ended with an error instead of the committed text. The echo instruction is now the final user turn, which every provider accepts; the upstream step-limit message is unchanged. Found by the live Bedrock smoke session, not by the mocked-provider tests. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): make the commit echo robust to output limits, queued turns and failed retries - A finished echo whose text hits the model's output limit, or differs from the stored committed output, is replaced with that output verbatim and finishes with `stop`; a correct echo is left untouched and an input overflow still takes the compaction retry. - Immediately before an echo is sent, persisted messages are re-read: a user request that arrived after the iteration's snapshot clears the commit, drops the unused assistant message and restarts the loop with tools, so a new question is never answered with the previous question's echo. - Re-detection looks at the last `answer_commit` attempt of any status; only a completed last attempt enters echo mode, so a success followed by a failed retry keeps the tools available for repair. - Prompt: the `StructuredOutput` precedence sentence now applies only when the request supplies an output JSON schema, not to text questions about schemas. - Tool description states that numbers are accepted up to 15 significant digits and that decimals which must be reproduced exactly are passed as strings. - Regression tests for the length-limited and mismatched echo, exact echo with a `length` finish, queued user turn before and during the echo, success-then- failure commit pair, prompt wording, and the description sentence. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): keep the full committed text through the per-result tool-output cap `SessionProcessor` caps every tool output to a model-dependent token budget before persisting it (a 4K-context model allows about 400 tokens), so a rendered answer longer than that was stored as a truncated preview and the echo logic reconstructed the committed text from that preview. The tool now also stores the complete rendered text in result metadata, which the cap does not touch, and the session loop reads the committed text from there (falling back to the output for older results). The model-facing output stays capped. Regression tests under `tool_output.dispatch_max_tokens: 100`: a 30-row commit whose persisted output is the capped preview still yields the full table as the final assistant text, and a short commit is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): deliver the committed echo to clients exactly once - The echo generation's text parts are buffered instead of streamed: the processor takes an opt-in buffer for echo mode and skips text start, delta, completion and the interrupted-text close, so clients never see an unvalidated echo. After the finish is validated the loop publishes one authoritative text part through the normal start, delta, completion sequence, which serves both the `run` output collector (completed parts) and ACP (deltas). A mismatched or length-limited echo therefore no longer prints the model's text and then the corrected text. - A `stop` finish with no text yields a committed text part with `time.start` and `time.end`, so `run` and `--output` no longer skip it. - Replacement and `stop` normalisation apply only to `stop` and `length` finishes; `other`, `error` and `content-filter` keep their text, timing and finish reason so `run` accounting still reports them. - Regression tests use real bus events, the real output collector with the `run` completed-text filter, and ACP-style delta collection: a mismatched echo delivers the committed output exactly once, a `stop` echo without text publishes a completed timed part, and an `other` finish is left untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): finish from the committed text on echo transport errors; drop the prompt copy; allow the echo turn under `--max-turns` - An echo generation that ends in a provider or transport error (API, unknown transport, provider auth; not an abort, content filter, context overflow or HTTP policy rejection) now publishes the stored committed output through the same single start, delta, completion path as a successful echo and finishes with `stop`. The error is kept on the assistant message as `metadata.answer_commit_echo_error` instead of a fatal error, and the processor skips the fatal session-error event for that case so headless accounting reflects the recovered answer. Assistant messages gain an optional `metadata` field in both schemas. - The echo instruction no longer appends the full committed output when compaction hid the tool part; the loop replaces the echo text from the stored output anyway, and the copy could overflow the post-compaction input. - `run --max-turns N` allows step N+1 when the previous counted work message's last `answer_commit` attempt completed, so the committed echo is not aborted as an over-budget turn; a run without a completed commit still aborts at N+1. Help text and snapshot updated. - Regression tests: non-retryable echo transport failure publishes the committed output once and records the error in metadata; aborted, content-filtered and HTTP-policy-rejected echoes keep their fatal error; compaction cases assert no full-answer copy in the request; `--max-turns 1` run publishes the committed output on its echo turn. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): deliver the committed text on an error-free `other` echo finish An error-free echo that finished with the provider reason `other` published the model's buffered text unchanged, so a provider mapping an unrecognised stop reason to `other` could replace the committed answer with a mismatched echo. Error-free, non-aborted `stop`, `length` and `other` echoes now all deliver the stored committed output; only `length` (and the transport-error recovery) normalise the finish to `stop`, so `other` stays reported to headless accounting. Aborts, content filters and unrecovered errors are unchanged. Test `an echo with an other finish keeps its finish and delivers the committed output exactly once` replaces the earlier original-text assertion. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): deliver the committed text when the echo finish is unmapped A finish reason outside the SDK's set is mapped to `unknown`. An error-free echo ending that way was excluded from committed-text delivery, and because the loop treats `unknown` as unfinished it then ran another tool-enabled generation instead of returning the committed answer. Such an echo now delivers the stored output like `stop`/`length`/`other` and ends as a normal `stop`, since the real reason is unknown and the text was delivered. Aborts, content filters and unrecovered errors are unchanged. The OpenAI-compatible adapter used by the agent tests maps every unrecognised provider reason to `other` (covered by the existing `other` scenario), so the `unknown` branch has no provider-level fixture here; it mirrors that branch and is covered by typecheck. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): bound `basis` by length, state the precision range, add per-column row precision Live sessions produced two avoidable `answer_commit` validation errors: the "at most two sentences" check split a basis on "U.S.", and a `precision` of 14 was rejected without the bound being visible to the model. `basis` is now bounded by `MAX_BASIS_CHARACTERS` (400) instead of a punctuation-based sentence count, and the description and validation message state the precision range (0 to 10). `rows` gains an optional `precision` map keyed by column name so numeric cells, including integers, render with fixed decimals; strings, nulls and unlisted columns are unchanged. Live sessions showed a numeric `5` rendered where the equivalent `"5.00"` was expected. Tests: two-sentence basis with "U.S." and "e.g." accepted; 400 characters accepted, 401 rejected naming the bound; precision range stated in the description and in the error for 14; row integers with precision 2 render `5.00`, without precision `5`, numeric strings keep trailing zeros, precision applies independently per column; the Anthropic draft-2020-12 schema test still passes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): one guard finalizes the committed answer on any non-abort exit; track the echo turn from the stream - `session/prompt.ts`: a single `finishCommittedAnswer` helper publishes the stored committed output through the start, delta, completion sequence, normalises the finish to `stop` and records the original cause in `metadata.answer_commit_echo_error`. The normal echo and the transport-error recovery use it, and a post-loop guard applies it when the request is about to end for any other non-abort, non-content-filter reason after a completed commit: a failing compaction summariser, an unrecoverable context overflow, a thrown preparation error. The guard re-checks for a newer real user request before publishing. Real request persistence emits `session.request.started`. - `cli/cmd/run.ts`: the `--max-turns` exemption no longer inspects the previous message. A completed `answer_commit` observed on the stream for the active request sets a flag that survives compaction and echo retries (so N+2 after a reactive overflow is still delivered); an explicit prompt, a synthetic follow-up, a soft-time reminder or another client's request clears it; a late completion from an older assistant cannot restore it. The remote message GET on the step-start path is gone. - `answer_commit`: every `rows.precision` key must name a column exactly; the error lists the unknown keys and the available columns. - Regression tests: failed compaction summariser, aborted and content-filtered compaction, unrecoverable overflow and thrown preparation error after a commit; `--max-turns 1` surviving reactive compaction with the echo on N+2; soft-time reminder clearing the exemption; precision key mismatch and exact match. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): recover the echo only from transient transport failures; never persist a transient fatal error - Recovery is structural, not phrase-based: an echo failure is recoverable only when it is an `APIError` with HTTP 5xx, 429 or 408, or a statusless network/stream failure (connection reset, socket timeout, idle watchdog, SDK retryable request failure). Any 4xx, any structured policy, content, safety or moderation code in the body, `ProviderAuthError`, and local errors raised before the provider request stay fatal. Error normalisation preserves the structured code ahead of body truncation and keeps a policy decision with overflow-looking text fatal. - A context overflow on the echo step after a completed commit is recoverable because the committed text already exists: the processor records the cause as `metadata.answer_commit_echo_error` instead of setting `error` or publishing `session.error`, and the loop publishes the committed text directly. This fixes the headless `--max-turns 1` scenario where the overflow was published as a fatal event before the recovery guard ran and the run exited 1. - The `--max-turns` exemption is cleared at every distinct persisted real-request boundary, including a request submitted by another attached client, and the step-start path never awaits a session fetch. - `rows.precision` with duplicate column labels is rejected; duplicates without precision render unchanged. - Tests: 503 and socket reset recover without fatal events; 400 structured policy denial and 403 stay fatal; local pre-request failure stays fatal; overflow on the echo publishes the committed text without fatal events; policy code beyond the body limit survives normalisation; over-budget step aborts without any session fetch; new attached-session request clears the exemption; `--max-turns 1` publishes the committed output after an echo overflow; unique column labels required for precision. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): deliver the committed answer even when a new request arrives during the echo; revoke the turn exemption on a failed retry - A user request persisted while the tool-free echo was streaming made the finalizer give up before publishing, so the model's unvalidated echo was published for the earlier request and the loop stopped without processing the new one. The committed answer belongs to the earlier request: it is now published once (start, delta, completion, finish `stop`), the commit is cleared, and the loop continues to answer the queued request with tools. The pre-send check for a request that arrived before the echo was sent is unchanged. - The `--max-turns` echo exemption now lives in the run accounting and is revoked when a later `answer_commit` attempt for the same request errors, so a success followed by a failed retry no longer exempts an ordinary work turn. - Tests: a user message persisted during a mismatched echo preserves the committed answer and is answered in the same prompt call; a failed `answer_commit` retry clears the committed echo exemption; a successful commit grants it; the existing during-echo and after-snapshot cases strengthened. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): order requests by persisted time and sequence, not message ids; finalize an echo that triggers compaction - `PromptInput.messageID` accepts caller-supplied strings, so every "newer request" decision that compared ids could miss a later request whose id sorts earlier and publish the old committed answer for the new question. Requests are now ordered by the message table's persisted creation time with the insertion sequence as tie-breaker; the finalization guard, the pre-send re-check, the queued-request reminder and the loop exit check (now by assistant parent) all use it. - A committed-answer echo whose step result is `compact` is finalized from the stored output (single publish, finish `stop`, cause recorded) instead of publishing the unverified buffered echo and retrying after compaction. - Tests: later caller-supplied id sorting before the committed request is detected at the next snapshot, after the snapshot, and still preserves the earlier echo with tools available; equal creation times fall back to the persisted sequence; an echo triggering reactive compaction publishes only the committed text without a continuation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): persist the committed text before finishing; classify policy codes explicitly; keep recoverable compaction failures non-fatal - The committed text part is published and persisted before the assistant message is marked finished, and the processor defers its own finished-message writes for buffered echoes, so an interruption between the two steps can no longer leave a finished echo without its answer. - Policy classification uses explicit codes (`policy_violation`, `content_filter`, `content_policy`, `content_policy_violation`, `guardrail_intervened`, `safety_*`, `moderation_*`), including nested orchestration guardrail codes carried in a 503, and never treats `content_length_exceeded` as policy: a provider 413 stays a context overflow and takes the compaction path. An existing overflow classification is never overridden by a policy code. - An error-free `other` echo publishes the committed text but keeps `other` as its finish so headless accounting still reports the abnormal stream. - When a committed answer exists and the compaction summariser fails with a recoverable transport error, the compaction path records the cause in metadata instead of a fatal error and skips `session.error`; the recovery guard publishes the answer and the run exits clean. Auth, policy and abort failures stay fatal. - Tests: text persisted before any finished-message update; 503 guardrail denial fatal during echo and compaction; structured policy codes override a recoverable status; 413 `content_length_exceeded` stays an overflow and runs compaction; `other` finish kept with committed text; 503 summariser failure after a commit publishes the committed output without fatal events; 403 summariser failure stays fatal. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * feat(analyst): `rows.format` for tabular answers; recover the committed answer when compaction yields no summary - `answer_commit`: `rows.format` selects `markdown` (default), `csv`, `tsv` or `lines`. CSV/TSV include the header row, quote cells containing the delimiter, quotes or newlines (RFC 4180, embedded quotes doubled), render null as empty and honour `rows.precision`; `lines` emits one row per line with cells separated by one space and no header. With `answer_only` the output is exactly the formatted table. The tool description tells the model to set the format when the request names one. - compaction: two textless summariser responses after a completed `answer_commit` for the current request record the cause as `answer_commit_echo_error` metadata instead of a fatal error, so the guard publishes the committed text; without a commit the existing `UnknownError` is unchanged. - tests: csv/tsv quoting, lines, precision in csv, markdown default, schema still draft-2020-12 valid, rendered-size cap after quoting; committed and uncommitted empty-summary compaction paths. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * feat(analyst): one `answer_commit` field per item the request asks for A compound request ("the repository and the most copied file") was committed as a single field under `answer_only`, so the second item never appeared and the terminal tool could not repair it. - `answer_commit` description: supply one field per requested item, in the order asked; never collapse a compound request into one field or add fields the request did not ask for - `fields` schema description states the same contract; validation unchanged (field count is not checkable without the request text) - analyst prompt: the final paragraph asks for one field per item the request asks for; otherwise byte-identical - tests: description and schema contract; prompt contract in `sql-validation-e2e.test.ts` Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): publish a recovered committed answer on a fresh text part when the summary already streamed A compaction summariser that streams partial text and then fails recoverably had already sent deltas for its text part; reusing that part id published the committed answer as another delta on it, and clients that do not treat a blank update as a reset showed the partial summary concatenated with the answer. - `finishCommittedAnswer` reuses a part id only for buffered (`deferText`) echo text; when the stored text was streamed it removes every stored text part and publishes under a fresh id - `run --output`: the collector tracks part ids and drops removed parts from the reply, the previous reply and the fallback; `run.ts` forwards `message.part.removed` - ACP: compaction-summary message text is no longer forwarded as agent chunks (ACP cannot retract chunks); clearing the summary flag on recovery lets the committed delta through exactly once; subscriptions that join mid-stream pick the flag up from the fetched message - tests: streamed partial summary + 503, and two empty summaries after streamed text, each deliver exactly the committed text to ACP and run output; collector removal cases; ordinary buffered echo still reuses its id and publishes once Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): structured policy codes win over overflow-looking text; never finalize onto a completed compaction summary - `message-v2.ts`: an explicit policy code (`guardrail_intervened` incl. nested orchestration codes, `content_filter`, `content_policy*`, `policy_violation`, `safety_*`, `moderation_*`) keeps the error fatal even when the message text looks like a context overflow; explicit overflow codes (`context_length_exceeded`, `content_length_exceeded`) and HTTP 413 stay overflows; policy metadata survives response truncation - `prompt.ts`: when the message that would carry a recovered committed answer is a finished, error-free, non-empty compaction summary (a user request was queued during compaction), the summary stays intact as the compaction boundary and the committed text is published once on a new assistant message with the committed request's parent, agent, mode, model, variant and path; failed/empty summariser recovery still converts its own summary message - tests: 503 guardrail with overflow text during echo stays fatal without publishing or compacting; queued user after successful compaction keeps the summary and gets tools after the new committed-answer message; 413 `content_length_exceeded` still compacts and resumes; 21 `fromError` cases for code vs text precedence Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): persist a recovered committed answer before any request queued during compaction The round-19 path that publishes a recovered committed answer on a new assistant message timestamped it with the current time, which sorted it after a user request queued during compaction, so later prompts replayed it as the reply to that newer request. The new message now takes the earliest newer user request's persisted `time_created` minus one millisecond (compaction markers and synthetic continuations excluded), completion never precedes creation, and the loop reloads history after the recovery so the queued request sees the answer in its original place. Test: persisted order is summary, recovered answer, queued user; the queued request's model history shows the committed text before its own turn. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): only the provider's own error code decides a context overflow The overflow check searched every nested `code` in the response body, so a diagnostic field such as `details.request.code: context_length_exceeded` could mask a top-level `policy_violation` and let the echo guard publish a committed answer against a provider denial. Overflow precedence now reads only `error.code` or the root `code` (plus HTTP 413); policy codes are still matched anywhere in the body. Tests: a nested diagnostic overflow code never masks a top-level policy denial; the explicit top-level overflow cases are unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): keep processing a request queued during a failed post-commit compaction When a user request was persisted while a post-commit compaction ran and the summariser then failed recoverably, the loop exited: the exit guard published the previous request's committed answer and resolved every waiting caller with it, so the queued request got a stale answer and was never processed. - the generation loop and its exit guard run inside a recovery loop: after a recovery, persisted order is re-read and a newer user request (compaction markers and synthetic continuations excluded) resumes processing in the same generation; callers resolve only after that request has its own reply - a converted failed summary carries the original committed request's `parentID`; the round-20 placement of a recovered answer before the queued request is unchanged - tests: a user queued during a 503 compaction failure, and during two empty summaries, gets tools and its own reply after the original committed answer; callbacks resolve with the queued reply; run/ACP observers see each text once Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): read the upstream error code inside the gateway wrapper when classifying an overflow Narrowing overflow detection to the provider's own code dropped the one place a real upstream code lives behind the LiteLLM / Altimate Base gateway: `error.provider_specific_fields.error.code` under a generic outer `error.code`. That path is now one of the three consulted (upstream wrapper, `error.code`, root `code`); arbitrary nested diagnostic fields are still ignored. Test: a wrapped upstream `context_length_exceeded` / `content_length_exceeded` is a `ContextOverflowError`; the nested-diagnostic and top-level cases are unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * refactor(session): keep the request-started event at module scope - `RequestStarted` is only used inside `prompt.ts`; it now lives as a non-exported top-level constant instead of extending the `SessionPrompt` namespace, per the package convention for namespace-private helpers. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): a hard time-budget deadline still delivers the committed answer - `run --time-budget` marks its hard-deadline abort (`x-altimate-run-budget: hard-deadline`), the session abort route turns that into a typed `RunBudget.HarnessDeadline` abort reason, and the committed-answer guard treats only that reason as non-fatal: the stored `answer_commit` text is published once through the existing path and the message records deadline provenance. A user cancellation keeps today's behaviour (no finalization). - The following budget-finalization prompt returns that persisted answer without a model turn or a second publication when the finalization directive and control are the synthetic ones and no user request intervened. - tests: deadline abort during the echo publishes the committed text exactly once; user cancellation does not finalize; budget finalization after a hard abort reuses the commit without a model turn and releases its trace binding; the CLI abort carries the deadline marker. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): the `--max-turns` echo allowance belongs to the request that committed - `run` tracked the committed answer as one global flag, so a request queued by another client between the echo dispatch and its `step-start` reset it and the exhausted-budget guard aborted the earlier request's permitted echo (a non-deadline abort, so the stored commit was never published). Commits are now tracked per request id through the existing `assistantRequests` ownership, and the guard checks the owner of the arriving step; a queued request neither clears nor inherits another request's allowance. Ordinary turn counting and refusal are unchanged. - `message-v2.ts`: the echo error helpers (`echoNetworkCodes`, `echoPolicyCodes` and their parsers) move to non-exported module scope per the package convention; no external imports existed. - test: a request queued before the committed echo's `step-start` leaves the echo to publish its committed text and is itself refused for the exhausted budget. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): budget finalization reuses a committed answer delivered while the hard deadline arrives - The finalization turn previously reused the stored commit only when the deadline reason was present at publication time; a deadline arriving during publication left `answer_commit_deadline` unset and the follow-up prompt regenerated. The delivered commit is now always marked (`answer_commit_finalized`) and the budget-finalization prompt reuses it without a model turn in both orders. - test: budget finalization reuses the committed answer when the hard deadline arrives during publication (abort issued from the publication hook, exactly once). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(analyst): request ownership survives compaction continuations; an explicit policy code beats a 413 status - The proactive compaction continuation is an internal user row without a parent request, so the per-request `--max-turns` echo allowance recorded no owner for the post-compaction echo and `--max-turns=N` could abort it and lose the committed answer. The prompt loop now publishes an awaited `session.request.continued` event naming the parent for compaction markers, continuations and replays; the run ownership helper follows it to the original request id. Real request boundaries keep their own ids; compaction itself is unchanged. - `MessageV2.fromError` no longer lets an HTTP 413 status override an explicit policy code: a `policy_violation` on 413 stays a fatal policy error instead of a recoverable overflow that would publish the committed output; a 413 without a policy code is still an overflow. - tests: ownership preserved across proactive compaction with no queued request, a request queued before the marker, and one queued during the summary; an ordinary request compacted after a committed echo keeps its own over-budget ownership; 413 with and without a policy code. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * refactor: export `isEchoTransportError` at module scope The predicate moves out of the `MessageV2` namespace to a flat top-level export, following the package's export convention. Callers in `prompt.ts`, `processor.ts` and the answer-commit tests import it directly. No behaviour change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: import `isEchoTransportError` directly in the message tests Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(answer-commit): infer column scale and accept finite decimals (#1516) * fix(answer-commit): infer column scale and accept finite decimals - Infer the maximum decimal scale per numeric column (by position) when `rows.precision` is omitted, so `5` and `4.18` render as `5.00` and `4.18` in markdown, csv, tsv and lines output. - Explicit `rows.precision` still overrides inference. - Drop the 15-significant-digit limit on numeric values; unsafe integers are still rejected and NaN/Infinity are still rejected by Zod. - Update the tool and schema descriptions and add regression tests. Part of #1481 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(answer-commit): ignore float-noise cells when inferring column scale - Cells with more than 6 decimals that do not survive 12-significant-digit rounding no longer raise the inferred column scale. - Noise cells render at the inferred scale, or in their own short form when no other cell sets a scale. - Add regression tests for a noise cell among clean cells, a noise-only column, and genuine high-scale values. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(answer-commit): keep genuine long decimals out of the float-noise rule - Treat a cell as float noise only when it is within 1e-7 relative error of its 6-decimal rounding, so float32 and binary artifacts are ignored but genuine long decimals set the column scale. - Add a regression test for genuine long decimals beside clean cells. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(answer-commit): limit float-noise detection to double and float32 artifacts Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * fix(answer-commit): keep decimals more than two ulps from a six-place rounding Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * fix(answer-commit): require every named item as its own field - State in the tool description and analyst prompt that each item the request names (an entity and a related item or count) must be its own field, since basis and prose are hidden under answer-only output. - Add a test pinning the guidance and the answer-only rendering of a complete field set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(answer-commit): keep the rounded scale of recognized float noise - A recognized float artifact now contributes the scale of the shortest value it is an artifact of, so a column mixing it with coarser cells keeps the intended digits. - A column containing only noise cells keeps its values as given. - Add a regression test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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.
Issue for this PR
Part of #1481
Partially fixes the issue: the two numeric rendering/validation defects are fixed here. The third reported defect (an answer element stored only in
basisis dropped whenanswer_onlyis true) is deferred, so the issue should stay open.Stacked on
feat/analyst-v10-commit(#1449, itself stacked on #1441).Type of change
What does this PR do?
Why:
answer_commitis the terminal tool that renders a final answer. Numeric tables came out with inconsistent scale, and valid finite decimals were rejected, forcing models to retry or stringify values.Root cause:
rows.precisionentry; other numeric cells went through plain string conversion, so a column holding5and4.18rendered with mixed scale.18.440000534057617) and safe 16-digit integers.Before -> after:
5and4.18, no precision:5/4.18->5.00/4.18(markdown, csv, tsv, lines). Scale is inferred per column position, so duplicate labels work; strings and nulls are unchanged.rows.precisionstill overrides inference (existing rounding behavior).18.440000534057617in fields or rows: rejected -> accepted (renders18.44with precision 2). Unsafe integers, NaN and infinities are still rejected; exact decimals beyond JS precision still need strings.Compound requests (follow-up commit): a model that found both an entity and a related item/count could keep the second only in
basis/prose, whichanswer_onlyhides, so it was dropped. Theanswer_commitdescription and analyst prompt now state that every named item must be its own field because basis and prose are not shown under answer-only output. This is guidance, not runtime enforcement: a generic validator would need a structured request-target contract (counting conjunctions or scanning basis for values is unreliable and would reject valid exact-format answers), so the issue stays open for that.How did you verify your code works?
cd packages/opencode && bun test test/altimate/tools/answer-commit.test.ts: re-run on the final head withtest/agent/answer-commit.test.tsand the wholetest/altimate/toolsdirectory: 931 pass, 0 fail.bun test test/altimate/tools(whole directory): 836 pass, 1 fail, 1 error. The failure isdatamate-list-integrations.test.ts(a 5s timeout case), which does not import or touchanswer-commitand fails identically when run alone; this PR changes onlyanswer-commit.tsand its test.bun run typecheckinpackages/opencode: passes, 0 diagnostics.bun run script/upstream/analyze.ts --markers --base feat/analyst-v10-commit --strict: passes, no upstream-shared files modified.git diff --check: clean.Number.MIN_VALUE, mixed safe-integer boundary with fractions, inferred padding exceeding the length cap); probed in isolation and they pass.Screenshots / recordings
N/A (no UI change).
Known limitation (follow-up)
Float-noise detection cannot tell a genuine double that is within two ulps of a six-decimal value (e.g.
1.0000000000000002) from an arithmetic artifact such as0.1 + 0.2; such a cell is excluded from column-scale inference and shown at the column scale. Artifacts below1e-6are not recognized as noise. Neither affects values that are exactly representable at six decimals or fewer. Callers who need exact digits pass strings.Checklist
🤖 Generated with Claude Code
Note
Medium Risk
Changes terminal answer rendering and numeric validation for the analyst agent; wrong inference could misformat tables, but scope is limited to answer_commit output paths with broad test coverage.
Overview
answer_commitnow infers per-column decimal scale for tabular answers whenrows.precisionis omitted, usingdecimal.jsso mixed cells (e.g.5and4.18) render with consistent fixed decimals across markdown/csv/tsv/lines. Float-noise detection treats common IEEE artifacts (e.g.0.30000000000000004) as shorter values so they do not inflate column scale; genuine long decimals are preserved.Numeric validation drops the 15-significant-digit cap and only requires safe integers and finite numbers—values like
18.440000534057617are accepted; exact trailing zeros or beyond-JS precision still belong in strings. Tool/schema copy documents inferred scale and updated number rules.Analyst guidance (prompt +
answer_commitdescription) stresses that every named item in a compound question must be its own field, becauseanswer_onlyhides basis—this is documentation only, not runtime enforcement.Tests expand coverage for inference, float noise, duplicate column labels by position, and the revised validation behavior.
Reviewed by Cursor Bugbot for commit ccc03c9. Bugbot is set up for automated code reviews on this repo. Configure here.