<!-- CURSOR_AGENT_PR_BODY_BEGIN -->
## Summary
Backend slice of P5.9g (KLAIR-2927, sub-issue of KLAIR-2906). The Google
Docs add-on /addon/review projection now mirrors every field the in-app
ReviewPanel (klair-client/src/screens/BoardDoc/components/ReviewPanel.tsx)
groups and filters on: check_area, BU-wide / per-product scope
(including the Other Products rollup), pass findings, and the
skipped / missing_sources / failed_sources chips. All inputs
already live on the persisted ReviewResponse; the projection was
simply dropping them.
FE half is a separate manual PR — out of scope for this slice.
## Why It's Needed
The add-on sidebar today groups findings by severity only. The in-app
ReviewPanel is significantly richer — it bins findings by check_area,
exposes a scope-filter chip row (BU-wide / per-product / Other
Products), keeps pass findings under a collapsible "passed" section,
and surfaces banners for skipped checks plus missing_sources /
failed_sources. The add-on FE is blocked on the payload carrying the
same data, and the add-on client is too thin to fetch the wizard section
list to classify scope itself. This slice ships the carry-forward
(additive) and classifies scope server-side so the FE half can plug in
without re-deriving anything.
## Changes
klair-api/routers/board_doc_router.py
- AddonReviewFinding gains 3 fields with safe defaults:
check_area: str = "", scope_key (pattern-pinned to
^(bu-wide|product:.+)$), scope_label.
- AddonReviewResponse gains 5 fields + a new tiny model:
skipped_checks, skipped_check_reasons, missing_sources,
failed_sources, scope_chips: list[AddonScopeChip]. Each defaults
to an empty collection.
- New AddonScopeChip(key, label) model — key pattern-pinned to
^(bu-wide|product:.+)$ so a malformed scope key is caught at the
Pydantic boundary rather than mis-grouping the FE chip row.
- _addon_review_from_results stops dropping pass findings so the
add-on FE can split them into its own collapsible "passed" section.
dismissed still drops. Score + summary stay OPEN-only —
pass/addressed do not move the score. Sort uses the same multi-key
comparator as the in-app compareReviewFindings
(severity → check_area → check_id → finding_id) so the projection
is deterministic and ordering matches the in-app ReviewPanel.
- New pure helper _addon_finding_scope(session, finding) -> (key, label)
classifies BU-wide vs per-product server-side. Tier-1: an explicit
finding.product_name wins (sub-$10M / name-skew products that have
no dedicated section). Tier-2: section-id lookup classifies a finding
as product-scoped iff that section's section_type is PRODUCT_DETAIL
or MINOR_PRODUCTS_SUMMARY — the latter using the literal
"Other Products" label matching the in-app
getScopeForSectionId. Orphan section_ids log a debug
breadcrumb keyed on check_id and fall back to BU-wide.
- New _addon_scope_chips(session, scoped_findings) builds the chip
row from non-pass findings only: BU-wide first, then product
chips (PRODUCT_DETAIL + MINOR_PRODUCTS_SUMMARY) in spec.sections
order, then a sectionless tail (sorted by label) for product scopes
whose product has no dedicated section. Returns [] when no product
chip is present — an intentional divergence from the in-app
listScopeFilterChips, which renders a lonely BU-wide chip in the
same case. The membership rule lives in a shared
_ADDON_PRODUCT_SECTION_TYPES frozenset + _addon_product_section_label
helper so the classifier and the chip builder can't drift.
- The needs_review branch in addon_review keeps its existing
behavior — the new fields fall back to their Pydantic defaults
(empty lists / dict).
klair-api/tests/board_doc/test_addon_review_enrichment.py (new)
18 tests covering the helper, the projection enrichment, the
needs_review branch, back-compat, MINOR_PRODUCTS_SUMMARY parity, and
twin-product-section dedup.
klair-api/tests/board_doc/test_addon_read_slice.py + test_addon_review_run.py
Updated existing assertions for pass no longer being filtered out
and for the new deterministic multi-key sort. Added a _bu_session()
helper for direct _addon_review_from_results calls now that it
takes session=. The POST /addon/review-run happy-path test now
asserts the same check_area / scope_key / scope_chips /
skipped_checks / missing_sources / failed_sources fields the
GET path does, so a future refactor that dropped session=session
from the run path would be caught.
## Breaking Changes
None — all additions are additive with defaults; existing field
names/semantics are unchanged. Old add-on clients that don't yet read
the new fields continue to work unchanged.
## Test Plan
cd klair-apiuv run pytest \
tests/board_doc/test_addon_read_slice.py \
tests/board_doc/test_addon_review_enrichment.py \
tests/board_doc/test_addon_review_run.py -v
44 tests pass (20 in test_addon_read_slice.py, 18 in
test_addon_review_enrichment.py, 6 in test_addon_review_run.py):
- check_area surfaced — test_projection_carries_check_area_on_each_finding.
- pass included / dismissed dropped / score+summary OPEN-only —
test_projection_includes_pass_drops_dismissed_score_open_only,
plus the updated
test_addon_review_maps_findings_for_owner and
test_addon_review_projection_preserves_source_finding_ids_and_mapping.
- skipped + sources passthrough — test_projection_carries_skipped_and_sources.
- scope classification — seven helper tests covering tier-1
product_name wins, tier-2 section_id resolution, BU-wide fallback
for non-product sections / missing section / null section_id,
defensive BU-wide when spec is None, and **MINOR_PRODUCTS_SUMMARY
→ product:Other Products** parity with the in-app
getScopeForSectionId.
- chip ordering — test_projection_scope_keys_and_chips_in_spec_order,
test_projection_pass_findings_do_not_inflate_chip_row,
test_projection_bu_wide_only_doc_has_empty_chip_row,
test_projection_sectionless_product_finding_emits_tail_chip,
test_projection_minor_products_summary_chip_in_spec_order (Other
Products chip lands in spec order, not the sectionless tail), and
test_projection_twin_product_sections_emit_one_chip (multi-region
product sections sharing a product_name collapse to one chip).
- needs_review defaults —
test_addon_review_needs_review_defaults_new_fields_empty.
- back-compat —
test_addon_review_back_compat_existing_fields_unchanged.
- POST round-trip — test_addon_review_run_happy_path_maps_and_persists
now asserts the new fields too (regression guard against
session=session ever being dropped from the run-then-project path).
uv run ruff format + uv run ruff check clean. uv run pyright
routers/board_doc_router.py reports 0 errors, 0 warnings.
## Verification Artifact
_addon_review_from_results.model_dump() on a session with two
product sections (Kandy, Brava), a sectionless Tiny product
finding, a BU-wide warning, a critical Brava finding, and a pass on
the financials section — plus a skipped check + missing/failed
sources. Truncated for the PR body:
{"google_doc_id": "doc-demo",
"state": "reviewed",
"score": 79,
"summary": "1 critical, 1 warning, 1 info open.",
"findings": [
{"finding_id": "f1", "severity": "critical", "section_id": "product_detail_brava",
"check_area": "Financials", "scope_key": "product:Brava", "scope_label": "Brava"},
{"finding_id": "f2", "severity": "warning", "section_id": "financials",
"check_area": "Plan-on-plan", "scope_key": "bu-wide", "scope_label": "BU-wide"},
{"finding_id": "f3", "severity": "info", "section_id": null,
"check_area": "Benchmarks", "scope_key": "product:Tiny", "scope_label": "Tiny"},
{"finding_id": "f4", "severity": "pass", "section_id": "financials",
"check_area": "Structure", "scope_key": "bu-wide", "scope_label": "BU-wide"}
],
"skipped_checks": ["C5.2"],
"skipped_check_reasons": {"C5.2": "missing PnL-by-product data"},
"missing_sources": ["pnl_by_product"],
"failed_sources": ["redshift_pnl"],
"scope_chips": [
{"key": "bu-wide", "label": "BU-wide"},
{"key": "product:Brava", "label": "Brava"},
{"key": "product:Tiny", "label": "Tiny"}
]
}
Notes from the artifact:
- pass finding present in findings, but the score stayed at 79
(1 critical × 15 + 1 warning × 5 + 1 info × 1 = 21 penalty;
pass + addressed do not move the score).
- scope_chips are ordered **BU-wide first, then Brava (spec-order
product section), then Tiny (sectionless tail)**.
- skipped_checks / skipped_check_reasons / missing_sources /
failed_sources are carried forward verbatim from the source
ReviewResponse.
<!-- CURSOR_AGENT_PR_BODY_END -->
<div><a href="https://cursor.com/agents/bc-60dccef2-bb82-4662-a59b-ccfbf4334f99"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a> <a href="https://cursor.com/background-agent?bcId=bc-60dccef2-bb82-4662-a59b-ccfbf4334f99"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> </div>
