Repository navigation
fix(agents): read three-digit comma groups in currency columns as thousands - #12228
Merged
Merged
Conversation
…roups Step 1 of 3: add failing tests for currency columns whose commas sit in three-digit groups, plus negative cases for short fractions, four-digit groups, zero-leading groups, id-like columns, and integers beyond int64. Refs #12224 Co-Authored-By: Antigravity (Gemini 4 Argon) <noreply@google.com>
…usands Step 2 of 3: settle an AMBIGUOUS_COMMAS column whose unit matches _CURRENCY and whose bare cells all match _COMMA_GROUP as whole numbers with comma thousands. Refs #12224 Co-Authored-By: Antigravity (Gemini 4 Argon) <noreply@google.com>
Step 3 of 3: document the currency comma thousands rule in parse.py and docs/concepts/agent-network.md, and add changelog fragment. Refs #12224 Co-Authored-By: Antigravity (Gemini 4 Argon) <noreply@google.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The currency rule ran before the file's own evidence, so a German fuel receipt (`Datum;Preis;Liter`, `1,789 €` beside `45,5`) read the price as 1789, where main reads 1.789. An unambiguous decimal-comma column now settles the file first, as before. Owner decision of 2026-10-22: a semicolon delimiter, or a column of dot groups or `1.234` shapes, also keeps the question, since a fuel price has three decimals. Review follow-ups: a docstring for `_retype_thousands_commas`, the shared `_dot_groups_only` check, a test of its own for the semicolon file, a test for the int64 fallback beside comma groups, the rating column checked in the currency cases, and the docs and fragment updated (#12228). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GdMSqLR5ww4ji74EUSmk9M
MarkusNeusinger
marked this pull request as ready for review
October 22, 2026 14:08
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Two decimal-comma safeguards miss valid evidence, allowing neighboring currency values to be multiplied by 1,000.
2 open issues
What changed in this PR
Updates the agent dataset parser for #12224 to interpret three-digit comma groups in currency columns as thousands, while preserving decimal-comma handling when the file provides contrary evidence.
Changes:
- Adds currency-based interpretation and shared comma-thousands conversion with an int64 overflow fallback.
- Adds regression tests and telemetry classification for the new warning.
- Documents the parsing rule and its exceptions.
| File | Description |
|---|---|
tests/unit/agents/data/test_warning_codes.py |
Tests classification of the currency warning. |
tests/unit/agents/data/test_parse.py |
Covers currency groups, decimal-comma safeguards, identifiers, and overflow. |
docs/concepts/agent-network.md |
Documents currency interpretation and exceptions. |
changelog.d/currency-thousands-commas.md |
Records the parsing correction. |
agents/anyplot/data/warning_codes.py |
Recognizes the new thousands-separator warning. |
agents/anyplot/data/parse.py |
Implements currency inference and shared conversion helpers. |
Review effort: Balanced
Copilot review of #12228. A column of dot groups kept as text because one value is past int64 recorded no group mark, so it was no evidence and the currency rule read `€ 1,789` beside it as 1789; the mark now counts, as it does for a column that fits. A comma column of zero-led groups (`0,125`) has the `1,234` shape but holds no thousands group, so it now keeps the question too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GdMSqLR5ww4ji74EUSmk9M
4 tasks done
MarkusNeusinger
added a commit
that referenced
this pull request
Oct 22, 2026
…#12229) ## Summary - Folds the friction of the first Gemini pilot (#12224, #12228) into the `gemini-task` skill: `gh issue view --json` and `gh api` for PR bodies (a bare `gh issue view` and `gh pr edit` fail here, Projects classic), issues that name the files the gates couple (`warning_codes.py` for a new parser warning) and list cases where the existing rules must still win, the changelog check run after the fragment is committed, one environment sync line with the `agents` extra, a hand-over written in the repository's PR format, and a fast-forward of a stale shared checkout before a take-over. - `verify-core` gives the same sync line (`--extra test --extra dev --extra typecheck --extra agents`); without `agents` the tests under `tests/unit/agents/` fail at collection. - `tools.changelog check` names a fragment that is on disk but not committed when it reports "no changelog fragment"; it still reads commits only, as CI does (new test in `tests/unit/tools/test_changelog_tool.py`). - Records the owner's wish of 2026-10-22 that Claude propose the Gemini route itself for longer tasks that need no credentials. ## Plan N/A. The owner asked on 2026-10-22 to fix what Gemini's report-back and the take-over of #12228 found. ## Test plan - [x] `uv run pytest tests/unit tests/integration -n auto` (12,269 passed) - [x] `uv run ruff check . && uv run ruff format --check .`, `uv run mypy api core` - [x] `uv run python -m tools.changelog check --base origin/main` - [x] Checked by hand: a bare `gh issue view 12224` exits 1 here, `--json title,body,comments` works 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01GdMSqLR5ww4ji74EUSmk9M --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.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
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.

Closes #12224
Summary
"$1,200","$950","$12,500", also1,200 USD,€ 1,200,CHF 12,500) read as 1.2, 950 and 12.5. It now reads as 1200, 950 and 12500 with the warning "commas read as thousands separators, as a currency amount has at most two decimals" (telemetry codethousands_separators). A cell with a one- or two-digit fraction ($1,25), a four-digit group or a zero-leading group keeps the column on decimal commas. The 2026-10-15 boundary stays: a dot number in another column is no evidence, and a bare1,200keeps its question.main. An unambiguous decimal-comma column settles the file first, as before. Owner decision of 2026-10-22: a semicolon delimiter, or a column of dot groups (7.184.003) or1.234shapes, also keeps the question, so a German fuel receipt's1,789 €stays 1.789._retype_thousands_commasis shared by the currency rule and the existing cross-column rule. A whole number past int64 now makes such a column text withBIG_INTEGERSon both paths, as_retype_thousands_dotsalready did, instead of a rounded decimal-comma float.Datum;Preis;Liter / 01.10.2026;1,789 €;45,5read 1789. Commit 7e3671c fixes that and adds the review follow-ups.Plan
Issue #12224 (revised 2026-10-22 after Gemini's report-back).
Test plan
uv run pytest tests/unit tests/integration -n auto(12,285 passed); new tests inTestThousandsGroupscover the seven acceptance cases, the files that keep their decimal comma (decimal-comma column, semicolon, dot groups,1.234shapes), an id column, and the int64 fallback with and without a currencyuv run ruff check . && uv run ruff format --check .,uv run mypy api coreuv run python -m tools.changelog check --base origin/mainmain: every file that speaks for decimal commas (the review's five inputs) gives identical CSV and warnings; only the currency columns changeNot changed on purpose (review notes): a column read as currency thousands is no evidence for another bare
1,250column (it keeps its question), and a currency named only in the header (price (€)) or in lower case (usd) is not detected.🤖 Generated with Claude Code
https://claude.ai/code/session_01GdMSqLR5ww4ji74EUSmk9M