Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions services/_shared/personas/code_reviewer/dispatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
review_reasoner_diff,
)
from voice_pack import VoiceSelection, entitled_voice
from review_types import EFFORTS
from review_types import EFFORTS, SEVERITIES
from personas.code_reviewer.dedup import (
dedup_findings, finding_key, parse_rule, prior_keys_from_comments,
rule_marker,
Expand Down Expand Up @@ -768,12 +768,14 @@ def _consolidated_agent_prompt(evaluation: CodeReviewEvaluation) -> str:
return block


# Derived from the shared vocabulary so a new effort level can never
# silently drop its chip (the Severity-partition-assert drift class).
_EFFORT_LABELS = {e: e.replace("-", " ") for e in EFFORTS}

# Closed caveman-inspired chrome chips (FLINT-density scanability).
# Identifiers and check *names* stay plain ASCII; Markings surface uses these.
#
# These two are hand-written, so they can drift from the shared vocabulary.
# `_chip_vocabulary_drift()` is what stops that, and it is asserted in tests
# against SEVERITIES/EFFORTS - a level added to review_types but not given a
# chip here would otherwise fall through `.get()` to the bare identifier, and
# nothing would say so.
_SEVERITY_CHIP: dict[str, str] = {
"critical": "💀 critical",
"high": "🔥 high",
Expand All @@ -786,6 +788,21 @@ def _consolidated_agent_prompt(evaluation: CodeReviewEvaluation) -> str:
}


def _chip_vocabulary_drift() -> dict[str, frozenset[str]]:
"""Vocabulary levels with no chip. Empty dict means no drift.

Returned rather than asserted at import so a drifted chip table degrades
to the bare identifier in production (ugly) instead of refusing to import
dispatch (fatal). The test is where it must fail loudly.
"""
return {
k: v for k, v in {
"severity": SEVERITIES - set(_SEVERITY_CHIP),
"effort": EFFORTS - set(_EFFORT_CHIP),
}.items() if v
}


def _severity_chip(severity: str) -> str:
return _SEVERITY_CHIP.get(severity, severity)

Expand Down
27 changes: 22 additions & 5 deletions services/webhook/tests/test_code_reviewer_dispatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -1950,11 +1950,28 @@ def test_agent_prompt_blocks_are_fence_breakout_safe():
assert "````" in summary


def test_effort_labels_derive_from_shared_vocabulary():
"""A new effort level can never silently drop its chip - labels derive
from review_types.EFFORTS."""
from review_types import EFFORTS
assert frozenset(cr_dispatch._EFFORT_LABELS) == EFFORTS
def test_chips_cover_the_whole_shared_vocabulary():
"""A level added to review_types must not silently lose its chip.

This guard used to assert against `_EFFORT_LABELS`, a derived dict that
NOTHING used - so it could never drift and the test could never fail. The
dicts actually rendered, `_SEVERITY_CHIP` and `_EFFORT_CHIP`, are
hand-written and had no guard at all: a new severity would have fallen
through `.get()` to the bare identifier with nothing to say so. The guard
was watching a decoy.
"""
assert cr_dispatch._chip_vocabulary_drift() == {}

# And it can actually fail - a guard nobody has seen go red is a guess.
from review_types import SEVERITIES
original = dict(cr_dispatch._SEVERITY_CHIP)
try:
cr_dispatch._SEVERITY_CHIP.pop(next(iter(SEVERITIES)))
assert cr_dispatch._chip_vocabulary_drift() != {}
finally:
cr_dispatch._SEVERITY_CHIP.clear()
cr_dispatch._SEVERITY_CHIP.update(original)
assert cr_dispatch._chip_vocabulary_drift() == {}



Expand Down
Loading