fix(attribution): fall through invalid numeric aliases

[stranske]

Closes #1045

Change attribution skipped valid numeric aliases after a missing or invalid field and accepted booleans as amounts. Both parsers now skip booleans and overflowing values; optional parsing continues through invalid and non-finite candidates, preserving the first finite value and existing defaults.

The report regression proves that a valid fallback delta participates in confidence assessment. For example, a missing primary delta followed by 24 against a calculated change of 25 now correctly yields Medium confidence. The prior test expectation that an invalid primary delta suppresses later aliases is updated to the issue's fallthrough contract.

Tasks

• Continue optional numeric parsing through missing, empty, invalid, boolean, overflowing, and non-finite candidates.
• Reject boolean and non-finite amounts in both parsers while preserving finite values, alias precedence, zero, and defaults.
• Add helper regressions and public report coverage in tests/test_change_attribution_report.py.

Acceptance Criteria

• uv run pytest tests/test_change_attribution_report.py -q: 92 passed.
• Direct call _optional_float({"delta": None, "daily_change": 42.5}, ("delta", "daily_change")) returns 42.5.
• Deliberately restoring return None in the optional parser's missing-value branch causes 11 failed, 81 passed; restoring the fix gives 92 passed.

Validation

• Original implementation with updated regressions: 21 failed, 71 passed.
• Black, Ruff, mypy for the two changed files and git diff --check: pass.
• Tests used the existing Python 3.12 environment with UV_NO_SYNC=1, UV_PROJECT_ENVIRONMENT set to that environment, and PYTHONPATH=src; imported module path confirmed this worktree.
• Evidence: /Users/teacher/.codex/automations/pd-workloop-resume/artifacts/20260912T1001Z/ (baseline.log, mutation.log, restored.log, direct-gate.log).

Handoff

Ready for review. Keepalive owns asynchronous CI and review follow-up; closer must recheck the unchanged head, required checks, full active review threads, and the seven-minute review window before merging.

[coderabbitai]

Warning

Review limit reached

Next included review available in 1 minute.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 78 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 74e001d2-27cf-465f-bb57-e87f4d108525

📥 Commits

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

📒 Files selected for processing (2)

• src/counter_risk/reports/change_attribution.py
• tests/test_change_attribution_report.py

---

_{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 #1052 | Agent: Codex | Iteration 1/12

Current State

Metric	Value
Iteration progress	[#---------] 1/12
Action	stop (tasks-complete)
Agent status	✅ ALL TASKS COMPLETE
Gate	success
Tasks	6/6 complete
Timeout	45 min (default)
Timeout usage	8m elapsed (18%, 37m remaining)
Keepalive	✅ enabled
Autofix	❌ disabled

🔍 Failure Classification

| Error type | infrastructure |
| Error category | unknown |
| Suggested recovery | Capture logs and context; retry once and escalate if the issue persists. |

[stranske-keepalive]

Keepalive Work Log (click to expand)

#	Time (UTC)	Agent	Action	Result	Files	Tasks	Progress	Commit	Gate
0	2026-09-12 10:08:25	Codex	wait (gate-cancelled-transient-transient)	skipped	—	0	6/6	—	cancelled
0	2026-09-12 10:09:16	Codex	wait (gate-cancelled-transient)	skipped	—	0	6/6	—	cancelled
0	2026-09-12 10:10:01	Codex	skip (needs-human)	skipped	—	0	6/6	—	cancelled
0	2026-09-12 10:18:36	Codex	skip (needs-human)	skipped	—	0	6/6	—	success
0	2026-09-12 10:28:02	Codex	skip (needs-human)	skipped	—	0	6/6	—	success
1	2026-09-12 11:09:47	Claude	run (verify-acceptance) retry	success	—	0	6/6	—	success
1	2026-09-12 11:26:59	Codex	stop (tasks-complete)	skipped	—	0	6/6	—	success
1	2026-09-12 11:27:35	Codex	wait (gate-not-success)	skipped	—	0	6/6	—	—
1	2026-09-12 11:35:14	Codex	stop (tasks-complete)	skipped	—	0	6/6	—	success
1	2026-09-12 12:36:33	Codex	stop (tasks-complete)	skipped	—	0	6/6	—	success
1	2026-09-12 12:37:17	Codex	wait (gate-not-success)	skipped	—	0	6/6	—	—
1	2026-09-12 12:44:25	Codex	stop (tasks-complete)	skipped	—	0	6/6	—	success
1	2026-09-12 13:27:43	Codex	stop (tasks-complete)	skipped	—	0	6/6	—	success
1	2026-09-12 13:35:35	Codex	wait (gate-not-success)	skipped	—	0	6/6	—	—
1	2026-09-12 13:43:15	Codex	stop (tasks-complete)	skipped	—	0	6/6	—	success

[github-actions]

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

[stranske]

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

Latest Gate summary:

Gate run: https://github.com/stranske/Counter_Risk/actions/runs/34687632106
Conclusion: cancelled
PR: #1052
Head SHA: b413aef2434c6c02103c8279c4a99b5400133ef4
Autofix attempts for this head: 8 / 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 numeric alias fallthrough in change attribution while rejecting invalid numeric values.

Changes:

• Reject booleans, overflow, and non-finite values.
• Preserve valid fallback aliases.
• Add parser and report-level regression tests.

File summaries

File	Summary
tests/test_change_attribution_report.py	Adds alias, fallback, and confidence regressions.
src/counter_risk/reports/change_attribution.py	Updates numeric parsing and fallback behavior.

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: 10 / 3

Latest Gate summary:

Gate run: https://github.com/stranske/Counter_Risk/actions/runs/34687676903
Conclusion: cancelled
PR: #1052
Head SHA: b413aef2434c6c02103c8279c4a99b5400133ef4
Autofix attempts for this head: 10 / 3
Fix scope: src/, tests/, tools/, scripts/, agents/, templates/, .github/
Failing jobs: none reported.

Please investigate manually.

[stranske]

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

Latest Gate summary:

Gate run: https://github.com/stranske/Counter_Risk/actions/runs/34687632637
Conclusion: cancelled
PR: #1052
Head SHA: b413aef2434c6c02103c8279c4a99b5400133ef4
Autofix attempts for this head: 10 / 3
Fix scope: src/, tests/, tools/, scripts/, agents/, templates/, .github/
Failing jobs:
- Python CI / typecheck-mypy (cancelled)
  - steps: Mypy (type check) (cancelled); Finalize mypy (failure)
- Python CI / python 3.12 (cancelled)
  - steps: Install dependencies (cancelled)
- Python CI / python 3.13 (cancelled)
  - steps: Cache uv artifacts (cancelled)

Please investigate manually.

[stranske]

Closer recovery audit for exact head b413aef2434c6c02103c8279c4a99b5400133ef4:

• The needs-human label was generated by cancelled Gate runs 34687632106, 34687676903, and 34687632637. Replacement Gate 34687679824 completed SUCCESS at 10:17:55Z on the same head, including Python 3.12/3.13, Ruff, mypy, and gate summary. No external human decision is required; the stale label has been removed.
• Full check inventory: 191 records, 57 names. Compared with 259 records on verified PR fix(renderers): align numeric table cells with headers #1051, the only missing name is verifier / Run post-merge verifier, intentionally post-merge. The configured lint-format skip is unchanged; the opener records Black PASS. All non-cancelled reported checks completed success/skipped. Remaining rollup FAILURE/UNSTABLE comes from cancelled Autofix bookkeeping (Resolve Context, Record autofix dispatch completion); Autofix 34687679843 was cancelled after 12 seconds. I am rerunning that cancelled workflow once to obtain a current terminal status instead of treating the cancellation as success.
• CodeRabbit reports a usage spending cap despite its SUCCESS status context, so that is not represented as substantive approval. The required fallback helper selected Orchestrator advisory review. Cursor completed an independent review: approve, no P0/P1 findings, focused 92 tests PASS and direct alias gate 42.5 on this exact head. The complete diff, source [P2] Fix alias fallthrough and reject booleans in change attribution float parsing #1045, original21fail/mutation11fail/restored92pass artifacts, and Copilot exact-head no-findings review were checked. The old invalid-primary-blocks-fallback behavior is intentionally replaced by the issue's fallthrough contract; no extra code change is needed.
• Main has no configured branch protection or rulesets, and no CodeRabbit-specific required status waiver is needed. Lane gates still apply. Full current review-thread read has zero active unresolved threads. The conservative seven-minute floor was 10:15:24Z; any later push restarts it.

Next action: inspect the rerun and unchanged head, refresh the complete status/review state, then merge and apply verify:compare only when the gates are satisfied. This is automation-owned recovery, with no request for human approval.

[stranske]

Runner dispatch state for autofix on PR #1052. Do not edit.

[github-actions]

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

[stranske]

Opener drain audit at unchanged head b413aef: full active review threads = 0; tasks and acceptance criteria complete; product Gate 34687679824 succeeded. Existing Cursor advisory review remains approve on this head, as documented in the preceding closer audit.

Autofix 34687679843 attempt 2 failed: the safe-sweep job leaves tracked node_modules changes/deletions and reaches git commit with no staged changes (exit 1). Evidence: https://github.com/stranske/Counter_Risk/actions/runs/34687679843/job/103540313796 . This is shared Workflows Autofix infrastructure, outside branch-local consumer recovery; there is no human decision or CodeRabbit blocker. Do not blindly rerun the same Autofix attempt or modify vendored dependencies in this implementation PR.

Dispatched one forced Gate Followups evaluation: https://github.com/stranske/Counter_Risk/actions/runs/34690169769 . Closer/workflow-health owns the exact Autofix staging/worktree failure and subsequent merge gate disposition. Opener continues unrelated rollup issue #1046 with capacity 1/8.

[stranske]

Automated exact-head review-window state. Each eligible merge wakeup re-observes the current head.

[stranske]

The failed Autofix run 34687679843 attempt 2 is now covered by a bounded source repair: stranske/Workflows#3420 . The reusable workflow was staging all changes, removing vendored node_modules from the index, then committing an empty index. The fix computes the consolidated change flag after staging/exclusions, so existing delivery guards skip empty commits and patches. Actual commit failures remain failures.

Validation: 35 focused tests pass; the original workflow fails seven new regression cases while three preservation controls pass, and the fixed workflow passes all ten delivery tests. Black/Ruff/actionlint and template checks pass. The full failed Actions log and current Workflows main were inspected. No consumer source or vendored dependency edit is needed, and the failed check has not been waived or blindly rerun.

Next automatic action: after Workflows #3420 passes its exact-head review/check window and merges, rerun this PR's Autofix once against the repaired reusable workflow. Then requery head, required/expected checks, mergeability and full active review threads; merge and apply verify:compare only after those gates pass. Source #1045 remains open. This is automation-owned, not a human decision. Next closer checkpoint: 2026-09-12T12:20Z or the next relay.

[stranske]

Upstream Workflows#3420 review repair pushed as3dd75bd97d339df7a4761a3df5151862556c69e8 at12:50:05Z. All six findings addressed and resolved after66focusedPASS, independent priorhead27FAIL/10controlsPASS/repaired37PASS, plus lint/actionlint/template checks. Fix covers vendor/helper filtering before validation, eligible file lists/report coherence and truthful patch availability.

This PR remains OPEN at b413aef. Its full252record check inventory still includes failed Autofix34687679843attempt2 despite aggregate SUCCESS; no successful replacement exists. The automerge label was removed but keepalive subsequently restored it; this is not merger authority and the failed check is not waived.

Next automatic action after3420 floor12:57:05Z: inspect its unchangedhead/fullchecks/fullthreads, merge only when clear, then rerun the ENTIRE failed Autofix workflow on1052 (not failed-jobs-only), confirm repaired reusable revision and disposition resulting artifact/check. Only then merge1052 and request verify:compare. Owner remains closer; next due13:20Z/next relay. No human decision is required.

The sixth finding was caught in the first new-head review: staged changes/deletions were invisible to the detectors. Four new cases fail against1af09c513 and pass after both detectors compare against HEAD with deletions included. Final66tests pass; latest3420head3dd75bd97 is ready, zero active threads, CI pending/no reported failures, no auto-merge.

[stranske]

Upstream Autofix repair Workflows #3420 merged at 13:29:51Z as 55075a2be16576012c69e5ef88e8c2c520d6ad60, after the exact-head review window, zero active review threads, passing required/full structural CI, and 41 fresh local delivery tests. The incorrect generated Workflows issue linkage was removed before merge. Its compare verifier run 34696597757 is in progress.

Reran the ENTIRE failed Autofix run 34687679843, now attempt 3, for unchanged consumer head b413aef2434c6c02103c8279c4a99b5400133ef4. GitHub referenced_workflows confirms reusable-18-autofix.yml@main binds to repaired revision 55075a2be16576012c69e5ef88e8c2c520d6ad60. Attempt 3 is in progress; attempt 2's failure has not been waived.

Next closer wake (due 2026-09-12T14:20Z or earlier relay): inspect attempt 3 and upstream verifier report, then re-read this exact head, full expected/required checks and complete active review threads. Merge only if all gates clear; apply verify:compare and disposition source issue 1045 afterward. No new implementation branch or consumer edits.

[stranske]

Final consumer merge audit for unchanged head b413aef2434c6c02103c8279c4a99b5400133ef4. The review floor 10:15:24Z elapsed hours ago; no head push occurred during the upstream repair.

Autofix run 34687679843 attempt 3 completed SUCCESS using repaired reusable-18 revision 55075a2be16576012c69e5ef88e8c2c520d6ad60. It supersedes attempt 2's actual failure with a successful rerun on the same consumer head. Full paginated check history: 313 records, 58 names, every latest check success/skipped. Current PR is OPEN/ready/CLEAN/MERGEABLE with aggregate SUCCESS and zero active unresolved reviewThreads (complete connection). No auto-merge is armed.

Re-audited the entire two-file diff and source #1045: both parsers reject booleans, overflow and nonfinite values; optional aliases continue to the first finite candidate. The public confidence regression checks real fallback behavior. Inspected original21FAIL/mutation11FAIL/restored92PASS and direct42.5 artifacts. Existing independent Cursor advisory and Copilot no-findings review both target this exact unchanged head; CodeRabbit's spending-cap SUCCESS context is not substantive approval and no new CodeRabbit waiver is claimed.

The previous absent-check comparison with verified #1051 identified only the post-merge verifier as absent. Current full inventory retains structural Python3.12/3.13, Ruff, mypy, Gate/summary and all expected names; configured lint-format skip is unchanged and opener Black passed. No required checks configured; lane gates still enforced. Source issue stays verifier-pending after merge and will close only on an audited durable report.

[github-actions]

Provider Comparison Report

Provider Summary

Provider	Model	Verdict	Confidence	Summary
openai	gpt-5.6-terra	PASS	95%	The attribution parsers now correctly fall through all candidate aliases when values are missing, blank/invalid, boolean, overflowing, or non-finite. _optional_float returns the later finite alia...
anthropic	claude-sonnet-5	PASS	86%	The core fix in change_attribution.py correctly resolves the alias-fallthrough bug: _optional_float and _first_float now continue (rather than short-circuit return None) on None/blank/invalid/b...

📋 Full Provider Details (click to expand)

openai

• Model: gpt-5.6-terra
• Verdict: PASS
• Confidence: 95%
• Scores:
  • Correctness: 10.0/10
  • Completeness: 10.0/10
  • Quality: 9.0/10
  • Testing: 9.0/10
  • Risks: 9.0/10
• Summary: The attribution parsers now correctly fall through all candidate aliases when values are missing, blank/invalid, boolean, overflowing, or non-finite. _optional_float returns the later finite alias value for the documented delta=None, daily_change=42.5 case, while _first_float and _optional_float both reject booleans and non-finite values without rejecting valid finite values including zero. The implementation catches OverflowError as required for extreme numeric conversions. Added regression and report tests cover alias fallthrough and invalid numeric candidates. The related limits and rollups adjustments consistently continue past blank aliases and reject booleans, with accompanying tests. No meaningful correctness, compatibility, security, or performance risks are evident from the changes.

anthropic

• Model: claude-sonnet-5
• Verdict: PASS
• Confidence: 86%
• Scores:
  • Correctness: 9.0/10
  • Completeness: 8.0/10
  • Quality: 8.0/10
  • Testing: 8.0/10
  • Risks: 7.0/10
• Summary: The core fix in change_attribution.py correctly resolves the alias-fallthrough bug: _optional_float and _first_float now continue (rather than short-circuit return None) on None/blank/invalid/boolean/non-finite/overflowing values, allowing subsequent alias keys to be checked. Both functions now explicitly reject bool instances and guard against OverflowError, matching the acceptance criteria described (e.g., _optional_float({"delta": None, "daily_change": 42.5}, ...) would correctly return 42.5). New tests were added to tests/test_change_attribution_report.py exercising these paths. Additional similar fixes were applied to limits.py and rollups.py (with accompanying new test files), extending the same fallthrough/boolean-rejection pattern beyond the strict issue scope — functionally reasonable and consistent, but a slight scope expansion worth noting. Implementation logic is sound, tests are present and appear adequate, and no obvious risks or regressions are introduced by the change itself. Post-merge CI results show all completed jobs as passing with no failures, supporting a PASS verdict.
• Concerns:
  • The fix scope was expanded beyond change_attribution.py to also modify _find_notional (limits.py) and _find_numeric (rollups.py), changing their behavior from raising ValueError on blank values to skipping to the next candidate key. This is a broader behavioral change than the issue explicitly scoped, and while it appears consistent with the alias-fallthrough fix pattern, it introduces additional surface area for regressions in limit-breach and rollup computations that isn't covered by the PR's stated acceptance criteria.
  • Diff truncation in the review context limited full visibility into all test assertions added for limits.py and rollups.py, so full confidence in their correctness relies on the stated additions (55 and 56 lines respectively) rather than direct verification.
  • Cannot directly execute uv run pytest tests/test_change_attribution_report.py -q to confirm the literal '92 passed' and '11 failed, 81 passed' deliberate-break gate numbers stated in the acceptance criteria; this is inferred from code logic rather than confirmed test run output in this review.

Agreement

• Verdict: PASS (all providers)
• Correctness: 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 8.5/10, range 8.0-9.0)

Disagreement

Dimension	openai	anthropic
Completeness	10.0/10	8.0/10
Risks	9.0/10	7.0/10

Unique Insights

• openai: The attribution parsers now correctly fall through all candidate aliases when values are missing, blank/invalid, boolean, overflowing, or non-finite. _optional_float returns the later finite alias value for the documented delta=None, daily_change=42.5 case, while _first_float and `_optional...
• anthropic: The fix scope was expanded beyond change_attribution.py to also modify _find_notional (limits.py) and _find_numeric (rollups.py), changing their behavior from raising ValueError on blank values to skipping to the next candidate key. This is a broader behavioral change than the issue explicitly scoped, and while it appears consistent with the alias-fallthrough fix pattern, it introduces additional surface area for regressions in limit-breach and rollup computations that isn't covered by the PR's stated acceptance criteria.; Diff truncation in the review context limited full visibility into all test assertions added for limits.py and rollups.py, so full confidence in their correctness relies on the stated additions (55 and 56 lines respectively) rather than direct verification.; Cannot directly execute uv run pytest tests/test_change_attribution_report.py -q to confirm the literal '92 passed' and '11 failed, 81 passed' deliberate-break gate numbers stated in the acceptance criteria; this is inferred from code logic rather than confirmed test run output in this review.

🔍 LangSmith Traces

• openai
• anthropic

[github-actions]

Workflow state fingerprint for Agents Verifier. Do not edit.

[stranske]

Verification and acceptance audit for source #1045 is complete. Verifier run 34696823272 succeeded on exact head b413aef2434c6c02103c8279c4a99b5400133ef4; the durable report records OpenAI PASS (95%) and Anthropic PASS (86%). The complete current review-thread connection has zero active findings.

The report's scope-expansion concern is a context error: this PR changes only src/counter_risk/reports/change_attribution.py and tests/test_change_attribution_report.py. Limits and rollups changes came through separate PRs #1054/#1053 with separately verified sources #1047/#1046. Full diff and current main confirm this PR's two parsers skip invalid aliases, booleans, overflow and nonfinite values while preserving finite zero, first-valid precedence and defaults.

Fresh validation in the original clean implementation worktree: UV_NO_SYNC=1 PYTHONPATH=src uv run pytest tests/test_change_attribution_report.py -q gives 92 passed. Current main's attribution file matches that tested file byte for byte. Inspected the original deliberate-break records: baseline 21 failed / 71 passed, missing-alias mutation 11 failed / 81 passed, restoration 92 passed, and direct example 42.5. The public report regression checks Medium confidence when a valid fallback delta is used. Prior mutation counts are inspected execution evidence, not newly rerun mutations.

Upstream Autofix repair Workflows #3420 is merged, verified dual PASS, and its truncated-diff/upload caveats have been dispositioned against the full workflow and tests. Consumer Autofix attempt 3 succeeded using the repaired reusable revision. All source tasks and acceptance criteria are satisfied; no remaining follow-up or human decision is identified. Closing #1045 after this durable verification audit.