fix(limits): traverse blank notional aliases and reject booleans

[stranske]

Closes #1047

Rows with a blank primary notional now use the next populated alias, so sparse exposure rows reach limit evaluation. Boolean notionals raise ValueError. Existing alias order, zero values, signed numbers, invalid-value rejection, and limit calculations remain unchanged.

Tasks

• In src/counter_risk/compute/limits.py, update _find_notional to continue checking remaining candidate keys when a candidate key is None or empty.
• In src/counter_risk/compute/limits.py, add boolean type guard not isinstance(val, bool) in _find_notional.
• In tests/compute/test_limits.py, add unit tests covering candidate fallthrough for {"notional": None, "exposure": 250000.0} and boolean input rejection.

Acceptance Criteria

• uv run pytest tests/compute/test_limits.py -q passes; fallback keys are evaluated when first key is None or empty.
• Direct inspection verifies _find_notional({"notional": None, "exposure": 250000.0}) == 250000.0.
• Deliberate-break gate: deliberately break _find_notional in src/counter_risk/compute/limits.py to raise on None without checking fallback keys; test must fail; restore and rerun.

Validation evidence

• uv run pytest tests/compute/test_limits.py -q: 43 passed. Includes public absolute and percentage breach calculations using sparse rows.
• Original implementation with these tests: 25 failed, 18 passed.
• Deliberate early-raise mutation: 14 failed, 29 passed; restored: 43 passed.
• Direct fallback call returns 250000.0. Black, Ruff, mypy, and git diff --check pass.
• Existing Python 3.12 environment reused with UV_PROJECT_ENVIRONMENT, UV_NO_SYNC=1, and PYTHONPATH=src; verified imports resolve to this worktree. Focused suite only.

The existing _NOTIONAL_KEYS tuple is preserved; the issue description names several aliases not currently supported. No alias expansion is needed for the stated acceptance criteria.

[coderabbitai]

Warning

Review limit reached

Next included review available in 2 minutes.

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: 4ea023bf-b7ee-4b81-96de-11997642cf95

📥 Commits

Reviewing files that changed from the base of the PR and between 8cc8b56 and b149e18.

📒 Files selected for processing (2)

• src/counter_risk/compute/limits.py
• tests/compute/test_limits.py

---

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

[github-actions]

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

[agents-workflows-bot]

🤖 Keepalive Loop Status

PR #1054 | Agent: Codex | Iteration 0/12

Current State

Metric	Value
Iteration progress	[----------] 0/12
Action	wait (gate-cancelled-transient)
Disposition	skipped (transient)
Agent status	✅ ALL TASKS COMPLETE
Gate	cancelled
Tasks	6/6 complete
Timeout	45 min (default)
Timeout usage	0m elapsed (1%, 45m remaining)
Keepalive	✅ enabled
Autofix	❌ disabled

🔍 Failure Classification

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

[agents-workflows-bot]

Keepalive Work Log (click to expand)

#	Time (UTC)	Agent	Action	Result	Files	Tasks	Progress	Commit	Gate
0	2026-09-12 12:08:23	Codex	wait (gate-cancelled-transient-transient)	skipped	—	0	6/6	—	cancelled
0	2026-09-12 12:09:03	Codex	wait (gate-cancelled-transient)	skipped	—	0	6/6	—	cancelled
0	2026-09-12 12:10:07	Codex	skip (needs-human)	skipped	—	0	6/6	—	—
0	2026-09-12 12:10:48	Codex	skip (needs-human)	skipped	—	0	6/6	—	cancelled
0	2026-09-12 12:17:43	Codex	skip (needs-human)	skipped	—	0	6/6	—	success
0	2026-09-12 12:34:13	Codex	wait (gate-not-success)	skipped	—	0	6/6	—	—
1	2026-09-12 12:44:26	Claude	run (verify-acceptance)	success	—	0	6/6	—	success

[github-actions]

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

[agents-workflows-bot]

🤖 Keepalive Loop Status

PR #1054 | Agent: Codex | Iteration 0/12

Current State

Metric	Value
Iteration progress	[----------] 0/12
Action	wait (gate-cancelled)
Disposition	skipped (transient)
Agent status	✅ ALL TASKS COMPLETE
Gate	cancelled
Tasks	6/6 complete
Timeout	45 min (default)
Timeout usage	1m elapsed (3%, 44m remaining)
Keepalive	✅ enabled
Autofix	❌ disabled

🔍 Failure Classification

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

[stranske]

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

Latest Gate summary:

Gate run: https://github.com/stranske/Counter_Risk/actions/runs/34692833899
Conclusion: cancelled
PR: #1054
Head SHA: b149e18e009ee016da462ec7850419575d8745c4
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

Validation passes and no unresolved issues were identified.

Pull request overview

Fixes notional extraction for sparse aliases and rejects boolean values before limit evaluation.

Changes:

• Falls through blank notional aliases.
• Rejects boolean notionals with ValueError.
• Adds focused and integration test coverage.

File summaries

File	Description
tests/compute/test_limits.py	Tests alias fallback, boolean rejection, and limit calculations.
src/counter_risk/compute/limits.py	Implements fallback 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.

[agents-workflows-bot]

🤖 Keepalive Loop Status

PR #1054 | Agent: Codex | Iteration 0/12

Current State

Metric	Value
Iteration progress	[----------] 0/12
Action	skip (needs-human)
Disposition	skipped
Agent status	✅ ALL TASKS COMPLETE
Gate	unknown
Tasks	6/6 complete
Timeout	45 min (default)
Timeout usage	0m elapsed (1%, 45m 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]

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

Latest Gate summary:

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

Please investigate manually.

[agents-workflows-bot]

🤖 Keepalive Loop Status

PR #1054 | Agent: Codex | Iteration 0/12

Current State

Metric	Value
Iteration progress	[----------] 0/12
Action	skip (needs-human)
Disposition	skipped
Agent status	✅ ALL TASKS COMPLETE
Gate	cancelled
Tasks	6/6 complete
Timeout	45 min (default)
Timeout usage	2m elapsed (6%, 43m 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 Loop Status

PR #1054 | Agent: Claude | Iteration 1/12

Current State

Metric	Value
Iteration progress	[#---------] 1/12
Action	run (verify-acceptance)
Agent status	✅ ALL TASKS COMPLETE
Gate	success
Tasks	6/6 complete
Timeout	45 min (default)
Timeout usage	10m elapsed (24%, 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 (Diff computed via git diff --name-only origin/main...HEAD: src/counter_risk/compute/limits.py, tests/compute/test_limits.py.) ## Criteria Status - [x] _find_notional continues checking remai...

🧠 Task Analysis

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

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

[stranske]

Closer audit on exact head b149e18: complete two-file diff and source #1047 acceptance inspected. Sparse alias fallback, boolean rejection, precedence and breach calculations match scope. Retained opener original25fail/mutation14fail/restored43pass evidence checked; Cursor Orchestrator fallback independently reran43 tests PASS and direct250000.0. Copilot recommended approval on this exact head; active review threads are zero.

CodeRabbit reported a usage spending cap, so its SUCCESS context is not treated as substantive review. The fallback helper was rechecked once and the prescribed advisory review completed approve/no P0/P1. Its optional alias expansion and non-Python boolean concerns are pre-existing and outside this issue’s explicit isinstance(bool) contract. Live branch protection returned404 and rulesets are empty; no separately required CodeRabbit approval needs a waiver.

The stale needs-human label came from cancelled Gate34692833899/34692834337, superseded by successful Gate34692904565 and Python34692904706 on this same head. Fully paginated148check records/57names have latest SUCCESS/SKIPPED only. Compared with1052, the absent nested autofix/autofix job is explained by its parent autofix being SKIPPED; lint-format is configured skipped. Runtime-AC merge guard has no additional required gate. Seven-minute review floor12:15:23Z has elapsed. Before merge I will requery exact head, full check inventory, mergeability and full active threads. Source1047 remains pending postmerge verify:compare.

[github-actions]

Provider Comparison Report

Provider Summary

Provider	Model	Verdict	Confidence	Summary
openai	gpt-5.6-terra	PASS	98%	The implementation satisfies the acceptance criteria. _find_notional now skips None, empty strings, and whitespace-only strings and continues through remaining aliases, so `{"notional": None, "...
anthropic	claude-sonnet-5	PASS	92%	The core fix in _find_notional correctly changes behavior so that None/empty candidate values now continue to the next key instead of raising immediately, satisfying the fallback requirement. A boo...

📋 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: 9.0/10
• Summary: The implementation satisfies the acceptance criteria. _find_notional now skips None, empty strings, and whitespace-only strings and continues through remaining aliases, so {"notional": None, "exposure": 250000.0} correctly resolves to 250000.0. It explicitly rejects boolean values before numeric conversion, preventing Python's bool-as-int coercion. Added tests cover blank-alias fallthrough across several aliases, boolean rejection, preservation of valid zero and numeric-string behavior, rejection when no usable alias exists, and integration through check_limits. The change is small, readable, and introduces no meaningful security, performance, or compatibility risk.

anthropic

• Model: claude-sonnet-5
• Verdict: PASS
• Confidence: 92%
• Scores:
  • Correctness: 9.0/10
  • Completeness: 9.0/10
  • Quality: 8.0/10
  • Testing: 9.0/10
  • Risks: 8.0/10
• Summary: The core fix in _find_notional correctly changes behavior so that None/empty candidate values now continue to the next key instead of raising immediately, satisfying the fallback requirement. A boolean type guard was added (isinstance(raw_value, bool)) that raises a ValueError before numeric coercion, correctly rejecting True/False inputs per the acceptance criteria. Direct inspection confirms _find_notional({'notional': None, 'exposure': 250000.0}) would return 250000.0 since the None value now falls through to check 'exposure'. Tests added in tests/compute/test_limits.py are thorough: they cover blank aliases with parametrized fallback keys, boolean rejection across multiple key names, precedence and zero-value handling, invalid value handling, and rows with no usable alias. An additional integration test validates check_limits end-to-end using fallback notional values for breach detection. This satisfies the deliberate-break gate requirement implicitly, since reverting the continue to a raise would cause the new fallback tests to fail immediately. Code is minimal, focused, and consistent with existing style. No security or performance concerns are introduced. Overall, both the implementation and test coverage align well with the acceptance criteria the PR and linked issue define.
• Concerns:
  • The final ValueError message when no usable alias is found is not shown in the truncated diff, but tests reference 'must include one of the notional columns', suggesting an existing fallback error path is reused correctly.
  • Boolean guard is placed after blank-check; since bool is a subclass of int, ensure True/False never pass isinstance checks elsewhere in the codebase for numeric coercion (not shown in diff, but logic here is correct).

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)
• Risks: scores within 1 point (avg 8.5/10, range 8.0-9.0)

Disagreement

No major disagreements detected.

Unique Insights

• openai: The implementation satisfies the acceptance criteria. _find_notional now skips None, empty strings, and whitespace-only strings and continues through remaining aliases, so {"notional": None, "exposure": 250000.0} correctly resolves to 250000.0. It explicitly rejects boolean values before...
• anthropic: The final ValueError message when no usable alias is found is not shown in the truncated diff, but tests reference 'must include one of the notional columns', suggesting an existing fallback error path is reused correctly.; Boolean guard is placed after blank-check; since bool is a subclass of int, ensure True/False never pass isinstance checks elsewhere in the codebase for numeric coercion (not shown in diff, but logic here is correct).

🔍 LangSmith Traces

• openai
• anthropic

[github-actions]

Workflow state fingerprint for Agents Verifier. Do not edit.

[github-actions]

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