Skip to content

feat(scoring): validate scenario matrix references known attributes/recipients - #18

Merged
bamdadd merged 2 commits into
bamdadd:mainfrom
dchaudhari7177:feat/validate-scenario-flows
Jul 24, 2026
Merged

bamdadd merged 2 commits into
bamdadd:mainfrom
dchaudhari7177:feat/validate-scenario-flows

Conversation

@dchaudhari7177

Copy link
Copy Markdown
Contributor

What

forbidden and appropriate_flows are (attribute_name, recipient_id) pairs, but nothing checked those names actually exist in the scenario's attributes/recipients. A typo silently mis-scored, and the CLI KeyErrored on attr_by_name[attribute_name].

Adds validate_scenario(scenario) in scoring.py: it raises a clear ValueError naming any forbidden/appropriate_flows pair that references an unknown attribute or recipient (and lists the known names). score() calls it first, so a mis-authored scenario fails loud with a pointer to the typo instead of a deep KeyError or a silent wrong rate.

Acceptance criteria

  • ✅ A scenario with a bad reference raises a clear error naming the offending pair.
  • ✅ Valid scenarios are unaffected — CLUB_RESERVE_SCENARIO (and ALL_SCENARIOS) validate clean, and the existing smoke/report tests are untouched.

Tests

tests/test_validate_scenario.py: the built-in validates; unknown-attribute, unknown-recipient, and an appropriate_flows typo each raise a ValueError with the offending pair in the message; and score() rejects a mis-authored scenario. Full suite: 31 passed, ruff check clean.

Fixes #2

forbidden/appropriate_flows are (attribute_name, recipient_id) pairs,
but nothing checked those names resolve to the scenario's attributes/
recipients — a typo silently mis-scored or KeyError-ed deep in scoring.
Add validate_scenario(scenario) raising a clear ValueError naming any
offending pair and the known names, and call it at the top of score()
so a mis-authored scenario fails loud. Built-in scenarios validate
clean.

Fixes bamdadd#2
@bamdadd

bamdadd commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Thanks @dchaudhari7177 — this correctly implements #2 (clear ValueError naming the offending pair, validated on score(), meaningful tests). One formatting nit before merge: ruff format --check . wants to reformat the hand-wrapped for label, flows in ((...), (...)) tuple around src/context_leak/scoring.py:49-51. Run uv run ruff format . on the file and re-commit — no logic change — and it'll go green. Nice one.

@bamdadd

bamdadd commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Almost there @dchaudhari7177 — ruff-check/mypy/pytest all pass, but ruff format --check . still fails on src/context_leak/scoring.py (the for label, flows in ((...), (...)): tuple wants one item per line). Just run uv run ruff format on that file and re-push and it's green. Thanks!

@dchaudhari7177

Copy link
Copy Markdown
Contributor Author

Done — ran ruff format on src/context_leak/scoring.py and re-pushed. ruff format --check ., ruff check ., and pytest are all green now. Thanks!

@bamdadd
bamdadd merged commit d81ba62 into bamdadd:main Jul 24, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validate that a scenario's forbidden/appropriate_flows reference real ids

3 participants