fix(rollups): traverse missing numeric aliases and reject booleans

[stranske]

Source: Issue #1046

Closes #1046

Automated Status Summary

Scope

src/counter_risk/compute/rollups.py:106 implements _find_numeric(row, candidates, ...). At line 119, when a row contains a candidate key whose value is None or whitespace, the loop executes break instead of continue. As a result, candidate traversal halts prematurely and raises a ValueError even when valid fallback candidate keys exist in candidates (e.g., {"notional": None, "exposure": 100.0} with candidates ("notional", "exposure") raises ValueError: Missing required numeric field instead of returning 100.0). Additionally, _find_numeric does not check isinstance(val, bool), allowing True to coerce to 1.0.

Tasks

• In src/counter_risk/compute/rollups.py, change break to continue in _find_numeric when candidate value is None or empty.
• In src/counter_risk/compute/rollups.py, reject boolean values in _find_numeric.
• In tests/compute/test_rollups.py, add tests verifying alias fallthrough when earlier candidate keys have None or empty string values, and verifying boolean inputs raise ValueError.

Acceptance criteria

• uv run pytest tests/compute/test_rollups.py -q passes; candidate keys fall through properly on None values.
• Direct inspection verifies _find_numeric({"notional": None, "exposure": 100.0}, ("notional", "exposure")) == 100.0.
• Deliberate-break gate: deliberately break _find_numeric by putting break back in src/counter_risk/compute/rollups.py; the alias fallback test must fail; restore and rerun.

Summary by CodeRabbit

• Bug Fixes

  • Improved numeric value handling so blank or missing aliases are skipped in favor of valid fallback values.
  • Boolean values are no longer treated as valid numeric inputs.
  • Alias precedence and required-value handling remain consistent.
  • Current and prior fallback aliases are now handled correctly when calculating totals.

• Tests

  • Added coverage for alias fallback behavior, boolean rejection, missing values, and total calculations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 96970d1b-a813-431e-8d3e-38bc86f7a399

📥 Commits

Reviewing files that changed from the base of the PR and between fc5301a and 81b9a3e.

📒 Files selected for processing (2)

• src/counter_risk/compute/rollups.py
• tests/compute/test_rollups.py

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

📝 Walkthrough

Walkthrough

The change updates numeric alias extraction to skip blank or None values, reject booleans, and continue to valid fallback aliases. Tests cover extraction behavior and compute_totals fallback handling.

Changes

Numeric alias handling

Layer / File(s)	Summary
Numeric extraction behavior src/counter_risk/compute/rollups.py, tests/compute/test_rollups.py	_find_numeric skips blank or None aliases, rejects booleans, preserves zero values and alias precedence, and raises errors when required values are missing.
Rollup fallback integration tests/compute/test_rollups.py	The tests verify that compute_totals uses fallback aliases for current and prior notional values.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 81b9a

The numeric alias fallback and boolean-rejection changes are covered without an identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Issue #1046 requires _find_numeric to continue after None or blank alias values, reject booleans, add regression tests, and preserve aggregation behavior. src/counter_risk/compute/rollups.py now…
Out of Scope Changes check	✅ Passed	The changes are limited to the requested _find_numeric behavior and tests in tests/compute/test_rollups.py. The aggregation implementation is not changed. The added compute_totals regression tes…
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main changes: continuing through missing numeric aliases and rejecting boolean values.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/issue-1046-rollup-numeric-aliases

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Workflow state fingerprint for Keepalive Loop Reporter. Do not edit.

[stranske-keepalive]

🤖 Keepalive Loop Status

PR #1053 | Agent: Claude | Iteration 2/12

Current State

Metric	Value
Iteration progress	[##--------] 2/12
Action	run (verify-acceptance)
Agent status	✅ ALL TASKS COMPLETE
Gate	success
Tasks	6/6 complete
Timeout	45 min (default)
Timeout usage	10m elapsed (23%, 35m remaining)
Keepalive	✅ enabled
Autofix	❌ disabled

Last Claude Run

Result	Value
Status	✅ Success
Changes	⚪ No changes

Claude output:

Verdict: PASS ## Scope Check - Files in diff: 2 - Files matching scope: 2 - Out-of-scope files: none ## Criteria Status - [x] uv run pytest tests/compute/test_rollups.py -q passes; candidate keys fall through properly on None values. - VERIFIED (evidence: CI "Python CI / python 3.12" and "python...

🧠 Task Analysis

| Provider | 🔶 Regex (fallback) |
| Confidence | 30% |

⚠️ Primary provider (GitHub Models) was unavailable; used Regex (fallback) instead.

[stranske-keepalive]

Keepalive Work Log (click to expand)

#	Time (UTC)	Agent	Action	Result	Files	Tasks	Progress	Commit	Gate
0	2026-09-12 11:11:07	Codex	wait (gate-cancelled-transient-transient)	skipped	—	0	6/6	—	cancelled
1	2026-09-12 11:13:50	Codex	run (bypass-rate-limit-gate)	success	32 file(s)	0	0/6	—	cancelled
1	2026-09-12 11:14:47	Codex	skip (needs-human)	skipped	—	0	0/6	—	cancelled
1	2026-09-12 11:20:37	Codex	skip (needs-human)	skipped	—	0	0/6	—	success
1	2026-09-12 11:26:45	Codex	wait (gate-not-success)	skipped	—	+6	6/6	—	—
2	2026-09-12 11:36:42	Claude	run (verify-acceptance)	success	—	0	6/6	—	success

[github-actions]

Workflow state fingerprint for Agents Gate Followups. Do not edit.

[github-actions]

Runner dispatch state for codex on PR #1053. Do not edit.

[stranske]

Autofix attempts exhausted for this head.
Attempts: 5 / 3

Latest Gate summary:

Gate run: https://github.com/stranske/Counter_Risk/actions/runs/34690342212
Conclusion: cancelled
PR: #1053
Head SHA: 81b9a3e2c18f2e40ee6ad77dfbe09dc92e3490f0
Autofix attempts for this head: 5 / 3
Fix scope: src/, tests/, tools/, scripts/, agents/, templates/, .github/
Failing jobs:
- classify changed paths (cancelled)

Please investigate manually.

[Copilot]

🟢 Approval recommended

No unresolved blocking issues were identified.

Pull request overview

Fixes rollup numeric alias fallback and rejects booleans as numeric values.

Changes:

• Continues past missing or blank aliases.
• Rejects boolean numeric inputs.
• Adds helper and aggregation regression tests.

File summaries

File	Description
tests/compute/test_rollups.py	Adds fallback, boolean rejection, and aggregation regressions.
src/counter_risk/compute/rollups.py	Updates numeric alias traversal and boolean validation.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[stranske]

Autofix attempts exhausted for this head.
Attempts: 6 / 3

Latest Gate summary:

Gate run: https://github.com/stranske/Counter_Risk/actions/runs/34690342734
Conclusion: cancelled
PR: #1053
Head SHA: 81b9a3e2c18f2e40ee6ad77dfbe09dc92e3490f0
Autofix attempts for this head: 6 / 3
Fix scope: src/, tests/, tools/, scripts/, agents/, templates/, .github/
Failing jobs:
- Python CI / python 3.12 (cancelled)
  - steps: Pytest (unit tests with coverage) (cancelled); Finalize check results (failure)
- Python CI / python 3.13 (cancelled)
  - steps: Pytest (unit tests with coverage) (cancelled); Finalize check results (failure)

Please investigate manually.

[stranske]

Closer premerge audit on 81b9a3e2c18f2e40ee6ad77dfbe09dc92e3490f0: the full two-file diff satisfies source #1046. Retained opener artifacts show original 17 failures, missing-alias mutation 13 failures, and restored 40 passes. Direct fallback is 100.0 with the existing required field keyword. CodeRabbit reviewed this exact head with no actionable findings; Copilot also found no blockers. Optional docstring coverage advice is outside this narrowly scoped acceptance contract.

The automated summary was overwritten with unchecked source tasks despite the delivered implementation; those six checkboxes are reconciled to the evidence. needs-human came from cancelled Gate runs 34690342212/34690342734 and a sandbox startup failure in a redundant runner, not missing product work. Replacement Gate 34690399026 passed Python 3.12/3.13, Ruff and mypy. The stale label is removed.

Fully paginated check inventory: 142 records, 60 names; latest record of every name is success/skipped. Compared with verified #1051, absent flat Fetch PR context and PR meta handler are represented by successful nested context/body-update jobs; verifier is post-merge only. No branch protection/rulesets are configured, so the explicit lane merge gates apply. The seven-minute floor was 11:17:58Z. Before merging, I will requery unchanged head, direct mergeability, full active review threads and check rollup. Source #1046 will stay open pending post-merge verify:compare disposition.

[github-actions]

Provider Comparison Report

Provider Summary

Provider	Model	Verdict	Confidence	Summary
openai	gpt-5.6-terra	PASS	98%	The implementation correctly replaces the premature break with continue, allowing _find_numeric to traverse later aliases when an earlier value is None, empty, or whitespace-only. It explicitly rej...
anthropic	claude-sonnet-5	PASS	90%	The diff correctly changes break to continue in _find_numeric when a candidate value is None or an empty/whitespace string, allowing fallback traversal to continue through remaining candidate...

📋 Full Provider Details (click to expand)

openai

• Model: gpt-5.6-terra
• Verdict: PASS
• Confidence: 98%
• Scores:
  • Correctness: 10.0/10
  • Completeness: 10.0/10
  • Quality: 9.0/10
  • Testing: 10.0/10
  • Risks: 10.0/10
• Summary: The implementation correctly replaces the premature break with continue, allowing _find_numeric to traverse later aliases when an earlier value is None, empty, or whitespace-only. It explicitly rejects bool values before float coercion, preventing True/False from being accepted as 1.0/0.0. Added tests cover None and blank alias fallthrough, multiple valid fallback values including zero and strings, boolean rejection, precedence/default behavior, and an integration-level compute_totals fallback case. The alias-fallthrough test would fail if continue were reverted to break, satisfying the deliberate-break gate in substance. The change is focused, readable, and presents no meaningful compatibility, security, or performance risk.

anthropic

• Model: claude-sonnet-5
• Verdict: PASS
• Confidence: 90%
• Scores:
  • Correctness: 9.0/10
  • Completeness: 9.0/10
  • Quality: 8.0/10
  • Testing: 9.0/10
  • Risks: 2.0/10
• Summary: The diff correctly changes break to continue in _find_numeric when a candidate value is None or an empty/whitespace string, allowing fallback traversal to continue through remaining candidate keys as required. It also adds an isinstance(raw_value, bool) check that raises a ValueError when a boolean is encountered, preventing booleans from being silently coerced to 1.0/0.0. New tests thoroughly cover alias fallthrough for None/whitespace values across various fallback types, boolean rejection with different defaults, precedence preservation (0 not falling through), and an end-to-end compute_totals scenario exercising both notional and prior_notional aliases. The implementation directly satisfies the acceptance criteria: _find_numeric({'notional': None, 'exposure': 100.0}, ...) == 100.0 is explicitly tested and should pass. The code change is minimal, readable, and low-risk, with no observed compatibility or performance concerns. Testing coverage is strong and aligns with the stated acceptance criteria including the boolean-rejection requirement, satisfying both correctness and completeness.
• Concerns:
  • Boolean values immediately raise ValueError rather than falling through to next candidate key; this is consistent with the added tests but slightly restrictive (a boolean in an earlier alias blocks fallback to a valid later alias). This matches the documented tests, so it's intentional, but worth noting as a design choice.
  • No explicit test that reverts the fix (break vs continue) was included in the diff itself; the 'deliberate-break gate' step is a manual verification step not encoded as an automated regression guard, though the accompanying tests would naturally catch a regression if the fix were reverted.

Agreement

• Verdict: PASS (all providers)
• Correctness: scores within 1 point (avg 9.5/10, range 9.0-10.0)
• Completeness: scores within 1 point (avg 9.5/10, range 9.0-10.0)
• Quality: scores within 1 point (avg 8.5/10, range 8.0-9.0)
• Testing: scores within 1 point (avg 9.5/10, range 9.0-10.0)

Disagreement

Dimension	openai	anthropic
Risks	10.0/10	2.0/10

Unique Insights

• openai: The implementation correctly replaces the premature break with continue, allowing _find_numeric to traverse later aliases when an earlier value is None, empty, or whitespace-only. It explicitly rejects bool values before float coercion, preventing True/False from being accepted as 1.0/0.0. Added...
• anthropic: Boolean values immediately raise ValueError rather than falling through to next candidate key; this is consistent with the added tests but slightly restrictive (a boolean in an earlier alias blocks fallback to a valid later alias). This matches the documented tests, so it's intentional, but worth noting as a design choice.; No explicit test that reverts the fix (break vs continue) was included in the diff itself; the 'deliberate-break gate' step is a manual verification step not encoded as an automated regression guard, though the accompanying tests would naturally catch a regression if the fix were reverted.

🔍 LangSmith Traces

• openai
• anthropic

[github-actions]

Workflow state fingerprint for Agents Verifier. Do not edit.

[github-actions]

Runner dispatch state for claude on PR #1053. Do not edit.