fix(compliance): make framework reports evidence-based and non-certifying

[parthrohit22]

Partially addresses #302 — see "Response to review" below for why this stays open rather than closing on merge.

Problem and security impact

Compliance reports could overstate assurance in ways that matter for anyone relying on them:

• A rule with zero completed scans ever run was reported as 100% compliant on every framework — get_compliance_score() derived PASS purely from "this rule_id is absent from today's failed-rule set," and an empty result set from no scan existing was indistinguishable from a scan ran and found nothing.
• Every one of the 95 rules was force-mapped into all four frameworks (CIS, NIST, ISO 27001, SOC 2) with no rationale, evidence type, or source — a rule providing only loose supporting evidence looked identical to a direct 1:1 technical check, and framework editions that predate a rule's subject matter (e.g. mapping post-quantum-crypto rules into 2013-2017-era frameworks) were mapped anyway.
• soc2.json and nist_csf.json had the same control_id recorded under multiple different control_name strings, and nist_csf.json mixed true CSF 1.1 subcategory codes with six SP 800-53 control codes (AC-17, CM-7, SC-5, SC-7, SC-8, SI-3) that don't exist in CSF.
• Historical scans had no record of which mapping-pack revision produced them, so an old report could be silently reinterpreted under whatever mapping file happens to be deployed later.

Response to review

ritiksah141 and TFT444 both reviewed this and found real correctness gaps. Addressed, in order of the numbered review:

1. CIS N/A-* entries classified direct (blocker) — fixed. All 46 CIS entries whose control_id starts with N/A-, or whose name/rationale said "not mapped"/"not directly mapped"/"no direct mapping," are now not_applicable. Added a CI check (.github/scripts/validate_mapping_pack.py) that rejects this combination going forward, so it can't regress silently. Not fully done: an individual, control-by-control audit of the remaining ~49 CIS entries still marked direct, and the independent security/compliance review of a representative sample, are both still outstanding — see "Limitations" below. Doing that audit honestly needs a human reviewer with domain judgment, not something I can self-certify in this PR.
2. Frontend converts "no evidence" into a zero score (blocker) — fixed. null is preserved end-to-end (api.js → Monitoring.jsx → ScoreGauge.jsx/FrameworkCards.jsx), status is propagated, and NO_SCAN_DATA/NO_IN_SCOPE_CONTROLS/a real score now render distinctly ("not assessed" instead of a false 0%/"Poor"). 9 new frontend tests in frontend/src/utils/api.test.mjs.
3. Historical snapshots don't reproduce historical mappings (blocker) — fixed. compliance_mapping_snapshot now captures each framework's full controls dict plus a content hash, not just pack metadata; get_compliance_score() reads controls from the snapshot (not the live file) when a full snapshot exists, and flags (rather than silently trusts) a hash mismatch. save_scan()'s ON CONFLICT no longer overwrites an existing snapshot on scan replay. Added the exact acceptance test requested: save under a v1 pack, change the live mapping to v2, requery the same scan, and prove controls/classification/denominator/hash/metadata all remain v1 (test_mapping_update_after_scan_does_not_change_that_scans_reported_mapping).
4. PASS still doesn't prove successful evaluation (blocker) — addressed via the second of the two options offered: until feat: persist PASS/FAIL/ERROR/NOT_APPLICABLE per rule per resource, fix compliance score #263 lands, scanner/engine.py now records failed_rule_ids for any rule that raised or returned malformed data, save_scan() persists it, and get_compliance_score() reports NOT_EVALUATED (excluded from the denominator) instead of reading a crashed rule's absence from findings as PASS. This is why the top line changed from Closes #302 to Partially addresses #302 per this item's explicit instruction — full closure of "PASS only from explicit successful evaluation" still needs feat: persist PASS/FAIL/ERROR/NOT_APPLICABLE per rule per resource, fix compliance score #263's per-resource persistence, since this only proves a rule ran, not that every resource it should have checked was reachable.
5. API contract/docs drift, in_scope_controls: 0 conflation — fixed. One response schema (status, score, max_score) documented and enforced across docs/api-reference.md, docs/architecture.md, docs/validation/FRONTEND_API_TESTING.md, frontend/API_ENDPOINTS.txt, docs/api-render-deploy.md; tests/test_score_route_contract.py added for route-level OK/NO_SCAN_DATA contracts; tests/smoke_test.py's null-unsafe TC-10/TC-11 fixed. NO_SCAN_DATA now returns null (not 0) for in_scope_controls/excluded_controls/passed/failed, plus its own evaluation_basis, so it can't be read as "a scan ran and found zero in-scope controls."
6. Silent snapshot failures — fixed. A framework file that's missing or malformed during snapshot capture is logged and recorded under _capture_errors in the persisted snapshot; get_compliance_score() reports mapping_provenance: live_fallback_capture_failed instead of silently presenting live data as historical. Tested for missing, malformed, and partial-failure cases.
7. Embedded CI YAML validation — fixed. Extracted into .github/scripts/validate_mapping_pack.py, a real importable module with 17 tests (tests/test_mapping_pack_validation.py), including one that runs it against the actual shipped framework files. ci.yml's CHECK 8 is now a single line calling it.
8. Alembic migration ordering — 3a76ff935bf6 (this PR) still shares down_revision = c7a2e9f1b3d4 with fix(core): enforce severity contract v1 #308's d8e4f6a1b2c3; fix(core): enforce severity contract v1 #308 remains open and unmerged as of this update, so no fork currently exists on dev. Added the CI assertion requested (alembic heads must return exactly one head), verified locally that it currently does. The fork can still only be resolved once one of the two PRs merges — whichever lands second rebases and repoints its down_revision, same as documented in the migration's docstring; the new CI check will fail loudly if that step gets missed instead of leaving multiple heads to surface later.

TFT444's two items are subsumed by the above: the migration-fork blocker is #8, and the get_score() empty-scan-returns-100 bug was the original item this PR set out to fix (get_score()'s NO_SCAN_DATA path, separate from get_compliance_score()'s).

Implementation summary

• compliance/frameworks/*.json (all 6 files): every control carries mapping_type (direct/supporting/organizational/not_applicable), evidence_type, primary_source, rationale, owner, review_status, review_date. Each file adds top-level mapping_pack_version/status/source/published. Fixed control_name inconsistencies in SOC2/NIST, remapped 6 SP 800-53 codes in nist_csf.json to correct CSF 1.1 subcategories, marked 9 AZ-PQC-* mappings not_applicable in pre-PQC-era frameworks, and reclassified 46 CIS N/A-* entries from direct to not_applicable (review item 1).
• api/models/finding.py: get_compliance_score() returns NO_SCAN_DATA (HTTP 200, all counts null, no error key) instead of computing from an empty result set; excludes not_applicable/organizational/NOT_EVALUATED controls from the denominator while still listing them; returns each control's mapping metadata, mapping_provenance, and an evaluation_basis string on every response state. save_scan() snapshots each framework's full mapping (controls + content hash) into scans.compliance_mapping_snapshot and preserves it immutably across replay. get_score() gets the same no-scan-data fix as get_compliance_score().
• scanner/engine.py: run_scan() now records failed_rule_ids for any rule that raised or returned non-list data, surfaced through to NOT_EVALUATED scoring (review item 4).
• alembic/versions/3a76ff935bf6_...: adds the nullable compliance_mapping_snapshot JSONB column. Verified upgrade head → downgrade -1 → upgrade head on a fresh local Postgres 16 database, plus the new single-head CI gate.
• .github/scripts/validate_mapping_pack.py (new): real, testable module extracted from CI's embedded heredoc; .github/workflows/ci.yml CHECK 8 now calls it.
• frontend/src/utils/api.js, ScoreGauge.jsx, FrameworkCards.jsx, Monitoring.jsx: preserve null/status end-to-end instead of coercing to 0.
• docs/compliance-mapping-pack.md: supported framework editions, mapping-pack schema, denominator-exclusion semantics, current (unreviewed) state of every mapping.

Scope decision: the #263 dependency

Issue #302's first acceptance criterion — "PASS is emitted only from an explicit successful evaluation" — depends on issue #263 (a persisted rule_evaluations table with per-resource PASS/FAIL/ERROR/NOT_APPLICABLE). #263 is still open with no schema or engine changes on dev. This PR implements the stopgap the review explicitly offered as an alternative to blocking on #263: NOT_EVALUATED derived from failed_rule_ids, which proves a rule ran but not that every resource it should have evaluated was reachable. Full closure needs #263's per-resource persistence.

Acceptance criteria checklist

• Depends on feat: persist PASS/FAIL/ERROR/NOT_APPLICABLE per rule per resource, fix compliance score #263 for full closure — NOT_EVALUATED stopgap implemented per review item 4; the remaining per-resource gap is stated in evaluation_basis on every response.
• Framework name, edition, mapping-pack version and source are persisted with every scan/report, immutably across replay — scans.compliance_mapping_snapshot (full controls + content hash), preserved via ON CONFLICT ... COALESCE.
• Current supported framework editions are documented; legacy packs are clearly versioned.
• N/A and organizational controls, and rules that didn't complete (NOT_EVALUATED), are excluded from technical pass-rate denominators.
• Every mapping has rationale, evidence type, primary source, owner and review date — owner/review_date remain null and review_status: "pending_review" throughout; see "Limitations."
• CI validates mapping semantics as a real, testable module, not embedded YAML — .github/scripts/validate_mapping_pack.py, 17 tests.
• Public copy no longer claims full compliance or certification.
• A sample mapping set receives independent security/compliance review before release — still not done; this is a human review step a PR cannot self-certify. See "Limitations."
• CIS entries individually reviewed for direct vs. supporting/organizational beyond the N/A-* fix — still framework-level for the remaining ~49 direct entries. See "Limitations."

Tests / checks run

Check	Command	Result
Compliance scoring	pytest tests/test_compliance_scoring.py -v	24 passed
Engine integration (failed_rule_ids)	pytest tests/test_engine_integration.py -v	all passed, including new malformed-data and always-a-list coverage
Mapping-pack validation	pytest tests/test_mapping_pack_validation.py -v	17 passed, including against real shipped framework files
Frontend score normalization	node frontend/src/utils/api.test.mjs	9 passed
Frontend build/lint	npm run build, npm run lint	pass
Full backend suite	pytest tests/ -q --ignore=tests/test_arg_inventory.py --ignore=tests/test_devops_client.py --ignore=tests/test_rag_dependencies.py	765 passed, 3 skipped (skips are pre-existing local-environment gaps unrelated to this PR)
Lint	ruff check .	All checks passed
Format	ruff format --check .	293 files already formatted
Migration	alembic upgrade head → alembic downgrade -1 → alembic upgrade head	Verified
Alembic head count	alembic heads	Exactly 1 head (3a76ff935bf6) — no fork against current dev
Rebase	git rebase upstream/dev	Clean, resolved conflicts from PR #316/#309/#287 landing on dev in the interim
DCO	sign-off on every commit	verified

Azure permissions / operational assumptions

None of this PR's changes touch Azure SDK calls or required permissions beyond scanner/engine.py now recording which rules failed to complete — no new Azure API interactions.

Limitations, unknowns, and follow-up

• #263 dependency — full closure of "PASS only from explicit successful evaluation" needs that issue's per-resource persistence; this PR's NOT_EVALUATED stopgap only proves a rule ran, not that every resource was reachable.
• Independent review is not done. Every mapping's review_status is "pending_review"; owner/review_date are null. CI's CHECK 8 rejects a "reviewed" entry missing either field, so this can't be faked.
• CIS mapping_type classification is still framework-level beyond the N/A-* fix. The 46 synthetic N/A-* entries are now correctly not_applicable; the remaining ~49 CIS entries are still direct by framework-level default, not individually audited per the review's specific ask. I don't have the standing to responsibly make that per-control judgment call without unfounded assumptions — the independent review step is the right place for it, and CI now prevents the exact regression this item was about (an N/A-*/"not mapped" entry silently becoming direct again).
• website/content.js still shows "NIST": "AC-17" for AZ-KV-002, mirroring the fixed nist_csf.json bug — left untouched as out of scope (separate static marketing content, no runtime dependency on the JSON files).
• Migration graph fork with #308 — no fork exists against current dev (fix(core): enforce severity contract v1 #308 unmerged); the new CI single-head check will catch it if that changes before one of the two merges.

No secrets or sensitive infrastructure data

Confirmed: no credentials, tokens, connection strings, IPs, hostnames, or other sensitive infrastructure data were added in this PR.

[TFT444]

Two blockers before this can land. First, migration 3a76ff935bf6 shares down_revision = 'c7a2e9f1b3d4' with PR #308's migration. Both cannot merge without creating an Alembic branch fork. Coordinate with the #308 author so one migration chains off the other. Second, the get_score() empty-scan-returns-100 bug is acknowledged in the PR description but left unfixed. This creates a false security posture and needs to be addressed here or tracked as a follow-up issue before merge.

[parthrohit22]

@TFT444 Both addressed:

• Migration conflict. Confirmed 3a76ff935bf6 and fix(core): enforce severity contract v1 #308's d8e4f6a1b2c3 share down_revision=c7a2e9f1b3d4. Chaining this one onto fix(core): enforce severity contract v1 #308's directly turned out not to be possible from this side alone — Alembic resolves the revision map from the files present in the branch it runs against, and pointing at d8e4f6a1b2c3 broke this PR's own CI (KeyError: 'd8e4f6a1b2c3') since that file only exists on fix(core): enforce severity contract v1 #308's unmerged branch. Documented the fork and the merge-time resolution in 3a76ff935bf6's docstring: whichever of fix(core): enforce severity contract v1 #308 / this PR merges second needs to rebase onto dev and repoint down_revision at the new head. Flagging this can't be fully closed from one side in isolation — happy to do that rebase myself the moment either PR lands.
• get_score() fixed, not just tracked. It had the identical bug: folding the completed-scan check into scan_id = (SELECT ...) can't distinguish "no completed scan" from "a completed scan found nothing," so absence-of-data silently reported a perfect 100. Restructured into two sequential queries (scan-existence check, then severity breakdown only if one exists) and it now returns {"status": "NO_SCAN_DATA", "score": null} instead. Added test_get_score_no_completed_scan_returns_no_scan_data_not_a_pass plus updated the existing get_score tests for the new two-query shape.

Full suite (723 passed, 3 skipped - pre-existing chromadb-under-3.14 skip, unrelated) and ruff are clean. PR description updated to match.

Latest commit: de75e40.

[ritiksah141]

Thanks for the substantial work here. The direction is correct—especially introducing NO_SCAN_DATA, mapping provenance, denominator exclusions, and non-certification language—but I found several correctness issues that must be resolved before merge.

1. Blocker: CIS direct mappings still overstate coverage

All 95 CIS entries are classified as direct, including entries whose IDs/names explicitly say they are not mapped to CIS, for example:

• AZ-SC-001 -> N/A-SC-001 — “not mapped in CIS Azure Foundations 2.0.0”
• AZ-SC-005 -> N/A-SC-005 — “not directly mapped”
• AZ-NET-016 -> N/A-NET-016 — “no direct CIS ... control”
• Multiple N/A-* backup, supply-chain, data-link, and security-operations entries

Because these are direct, they remain in the denominator and become PASS when absent from findings. This recreates the overstatement this PR is intended to prevent.

Required changes:

• Classify synthetic N/A-* entries as not_applicable.
• Use supporting for partial technical evidence and organizational where a scan cannot establish the control.
• Individually review mappings whose rule and control do not evaluate the same condition; do not classify the whole CIS file as direct by default.
• Add CI validation that an N/A-* control ID, or a name/rationale saying “not mapped”/“no direct mapping,” cannot be direct.
• Complete and record the acceptance criterion requiring independent security/compliance review of a representative mapping sample.

2. Blocker: the frontend converts “no evidence” into a zero score

The backend correctly returns score: null/score_percent: null, but frontend normalization uses nullish fallback to 0:

raw.score ?? raw.score_percent ?? 0
data.score_percent ?? 0

This causes no scan data to render as 0, 0%, and Poor. It also renders an all-excluded framework as 0%. That replaces a false-positive score with a false-negative score.

Required changes:

• Preserve null and propagate the backend status.
• Render Not assessed/No scan data rather than a gauge, percentage, trend point, or Poor label.
• Distinguish NO_SCAN_DATA, NO_IN_SCOPE_CONTROLS, and a genuinely evaluated numeric score.
• Add frontend tests for these three states.

3. Blocker: historical mapping snapshots do not reproduce historical mappings

compliance_mapping_snapshot stores only pack metadata. get_compliance_score() still loads controls, mapping types, and denominator membership from the current live JSON, then labels the result using the old snapshot metadata.

After a mapping update, an old scan can therefore be evaluated with v2 controls while claiming v1 provenance. In addition, save_scan() overwrites compliance_mapping_snapshot during ON CONFLICT, so replaying a scan after a pack update mutates its historical identity.

Required changes:

• Either snapshot the complete normalized mapping used for the scan, or persist an immutable content hash/version reference that can retrieve the exact historical pack.
• Preserve the original snapshot/reference on idempotent scan replay; do not overwrite it silently.
• Store and validate a content hash in addition to a human-maintained semantic version.
• Do not silently fall back to live mappings for a response presented as historical.
• Add a test: save with v1, change live mappings to v2, query/replay the v1 scan, and prove its controls, classifications, denominator, hash, and metadata remain v1.

4. Blocker: PASS still does not prove successful rule evaluation

The current scan engine catches individual rule exceptions and still completes the scan. get_compliance_score() treats every rule absent from findings as PASS, so a failed, skipped, timed-out, or permission-denied rule can still become a pass.

The evaluation_basis disclaimer is useful but does not make the numeric score or PASS evidence-based.

Required resolution:

• Prefer merging #263 first and derive PASS only from persisted successful rule/resource evaluations; or
• Until #263 exists, return UNKNOWN/NOT_EVALUATED for absence where successful evaluation cannot be proven and exclude it from the pass denominator.

Until this is resolved, please change Closes #302 to a partial/reference relationship and keep #302 open.

5. API contract and documentation need to move together

get_score() changes from an integer to an object, but repository documentation and smoke tests describe conflicting contracts. The new successful response also omits max_score, while examples expect it. No-data smoke-test comparisons are not null-safe.

Required changes:

• Define one response schema containing at least status, score, and max_score.
• Update API reference, frontend endpoint documentation, architecture/validation documentation, and smoke tests.
• Add route-level contract tests for OK and NO_SCAN_DATA.
• Preserve compatibility or version the endpoint if external consumers rely on the bare-number response.

The compliance NO_SCAN_DATA response should also include a consistent evaluation_basis and distinguish mapping composition from evaluation counts. in_scope_controls: 0 currently conflates “not evaluated” with “no in-scope mappings.”

6. Snapshot failures must not be silent

_build_compliance_mapping_snapshot() silently omits unreadable or invalid framework files. That can produce incomplete provenance and later fall back to live data.

Required changes:

• Log and persist snapshot completeness/error state.
• If mapping provenance is required for a completed scan, fail closed rather than silently omitting it.
• Test missing, malformed, and partially readable mapping packs.

7. Move semantic validation out of embedded CI YAML

The validation logic is valuable, but an 84-line Python program embedded in workflow YAML is difficult to unit-test and reuse.

Required changes:

• Move it to a repository script/module and invoke that from CI.
• Add invalid fixtures covering semantic versions, ISO dates, pack status, N/A-*/mapping-type consistency, evidence-type consistency, reviewer/date requirements, and content hash generation.

8. Alembic migration ordering must be resolved before merge

PR #308 remains open and its migration shares down_revision = c7a2e9f1b3d4. If both merge unchanged, the repository will have multiple heads.

Required changes/process:

• Establish merge order explicitly.
• Rebase the second PR and chain its migration to the new head.
• Add a CI assertion that alembic heads returns exactly one head.
• Re-run upgrade, downgrade, and upgrade against a populated database after rebasing.

Re-review checklist

• CIS and other framework mapping classifications are individually defensible.
• Synthetic N/A-* mappings cannot enter the technical score denominator.
• Independent sample review is recorded.
• Frontend preserves and visibly represents no-data/null states.
• Historical mapping results are immutable and reproducible.
• A rule cannot become PASS without successful evaluation evidence.
• /api/score and compliance response contracts, docs, frontend, and tests agree.
• Snapshot failures are explicit and fail safely.
• Mapping validation is testable outside workflow YAML.
• Alembic has exactly one head after merge-order coordination.
• New regression tests and the full CI/security suite pass.

Once these items are addressed, the PR will provide a much stronger foundation for trustworthy compliance reporting and the planned remediation automation.

[parthrohit22]

Thanks both for the thorough reviews — @ritiksah141's 8-item breakdown and @TFT444's two blockers caught real gaps, not nitpicks. Pushed a set of commits addressing them; here's what changed against each item (full detail in the updated PR description above):

1. CIS N/A-* classified direct — fixed for all 46 synthetic entries, plus a CI check so an N/A-*/"not mapped" entry can't become direct again. Still open: the individual per-control audit of the remaining ~49 CIS entries and the independent review sample — both need a human reviewer, not something I can self-certify here.
2. Frontend coerces null score to 0 — fixed, null/status preserved end-to-end with tests for all three states (OK/NO_SCAN_DATA/NO_IN_SCOPE_CONTROLS).
3. Snapshots don't reproduce historical mappings — fixed. Full controls + content hash now snapshotted, ON CONFLICT no longer clobbers on replay, hash mismatches are flagged not silently trusted. Added the exact acceptance test asked for (save v1, mutate to v2, requery, prove v1 held).
4. PASS doesn't prove evaluation — implemented the NOT_EVALUATED stopgap option (rather than pulling feat: persist PASS/FAIL/ERROR/NOT_APPLICABLE per rule per resource, fix compliance score #263 in whole): scanner/engine.py now records which rules failed to complete, and those are excluded from the pass denominator instead of reading as PASS. Changed Closes #302 to Partially addresses #302 per this item's instruction — full closure still needs feat: persist PASS/FAIL/ERROR/NOT_APPLICABLE per rule per resource, fix compliance score #263's per-resource persistence.
5. API contract/docs drift, in_scope_controls: 0 conflation — one documented schema everywhere, route-level contract tests added, and NO_SCAN_DATA now returns null (not 0) for the count fields so it can't be misread as "zero in-scope controls."
6. Silent snapshot failures — logged and recorded under _capture_errors; get_compliance_score() reports live_fallback_capture_failed instead of quietly presenting live data as historical.
7. Embedded CI YAML validation — extracted to .github/scripts/validate_mapping_pack.py with 17 tests.
8. Alembic ordering — fix(core): enforce severity contract v1 #308 is still open/unmerged so there's no fork against dev right now; added the alembic heads single-head CI gate you asked for so this can't silently regress once one of the two merges.

Also rebased onto current dev (picked up #316/#309/#287 landing in the meantime) and re-ran the full suite (765 passed, 3 skipped — pre-existing local-env-only skips) plus lint/format/build.

Re-requesting review from both of you — happy to keep iterating on anything I read wrong.

[Vishnu2707]

Seems like all of the concerns are addressed, kindly approve this @ritiksah141, @TFT444 .

[ritiksah141]

@parthrohit22, please resolve the conflicts and then do this 1. Rebases #310 onto the updated dev.
2. Changes migration 3a76ff935bf6 to:

down_revision = "d8e4f6a1b2c3"

3. Resolves any conflicts in api/models/finding.py, CI, frontend API utilities and tests.
4. Runs:

alembic heads
alembic upgrade head
alembic downgrade -1
alembic upgrade head

5. Confirms alembic heads returns exactly one head.
6. Resolves the requested changes already submitted on fix(compliance): make framework reports evidence-based and non-certifying #310. and ready for merge

[parthrohit22]

@ritiksah141 Done, all six steps:

1. Rebased onto updated dev.
2. 3a76ff935bf6's down_revision → d8e4f6a1b2c3. This is exactly the merge-time resolution documented in that migration's own docstring back when fix(core): enforce severity contract v1 #308 was still unmerged — updated the docstring to record that it's now resolved.
3. Conflicts resolved in api/models/finding.py, .github/workflows/ci.yml, and the frontend API tests/docs. finding.py needed the most care: reconciling fix(core): enforce severity contract v1 #308's atomic/idempotent save_scan() + score_counts() restructuring with this PR's compliance-mapping-snapshot/NOT_EVALUATED/evidence-schema logic in save_scan(), get_score(), and get_compliance_score(). CI keeps both new test steps (severity contract + score/compliance null-state). Docs and tests updated for the merged shapes.
4. Ran the full cycle against a local Postgres instance: alembic heads → single head → alembic upgrade head → alembic downgrade -1 → alembic upgrade head → alembic heads.
5. Confirmed: alembic heads returns exactly 3a76ff935bf6 (head), one head.
6. Already-requested changes — no new ones outstanding since the review round on the 24th; this pass was purely the rebase/conflict resolution above.

Two real issues turned up while reconciling the two branches' code, fixed rather than deferred:

• Silent bug in my own merge: get_compliance_score()'s severity/category grouping query briefly rode along on the RealDictCursor this PR uses elsewhere in the same method. RealDictRow has no __iter__ override, so for rule_id, severity, category, count in rows silently unpacks the dict's keys, not the values — no exception, just wrong data. fix(core): enforce severity contract v1 #308's own merged version of that query uses a plain cursor for exactly this reason; restored that instead of carrying the bug forward or unilaterally rewriting fix(core): enforce severity contract v1 #308's already-tested code.
• Pre-existing gap in an unrelated merged PR: validate_mapping_pack.py flagged AZ-CMP-007 (added by feat: add rule AZ-CMP-007 management ports open without JIT VM access #307, before this PR's evidence-schema requirement existed) as missing mapping_type/evidence_type/etc. across all four framework files. Filled them in following the conventions of the neighboring entries in each file.

Verified: full pytest suite (818 passed, 3 skipped), ruff check + format, mapping-pack validator, full frontend test/lint/build — and PR #310's CI is now green end-to-end (all 20 checks passing on 43ea824).

[ritiksah141]

All good to me. Approving it again

[parthrohit22]

@TFT444 Following up on your review from the 23rd — both items you flagged were addressed the next day:

• Migration fork: 3a76ff935bf6 and fix(core): enforce severity contract v1 #308's d8e4f6a1b2c3 did share down_revision = c7a2e9f1b3d4. Documented the fork and the merge-time resolution in the migration's docstring at the time, and since fix(core): enforce severity contract v1 #308 has now merged, this branch has been rebased onto it and down_revision repointed to d8e4f6a1b2c3 directly — alembic heads confirmed back to exactly one head.
• **get_score() empty-scan-returns-100**: fixed the same day — restructured into a scan-existence check followed by the severity breakdown only if one exists, so it now returns {"status": "NO_SCAN_DATA", "score": null}instead of a false 100. Covered bytest_get_score_no_completed_scan_returns_no_scan_data_not_a_pass`.

Since then this also went through ritiksah141's full 8-item review (all addressed) and their re-approval today on the current head (43ea824), plus the dev rebase and alembic chain repoint. Full suite (818 passed, 3 skipped), ruff, mapping-pack validation, and all 20 CI checks are green on that head.

Your review is still showing as the standing CHANGES_REQUESTED from before either fix landed, which is holding up merge. Could you take another look at the current head and re-approve or flag anything still open? Formally re-requesting your review as well so it lands in your queue.

[m-khan-97]

@TFT444, both blockers from your earlier review now have concrete fixes on the current head: the migration is chained after #308 with a single Alembic head, and the empty-scan score returns NO_SCAN_DATA rather than 100. Ritik has approved and all 20 checks pass. Please rereview the current head and either approve it or identify any remaining specific blocker so this does not stay held by stale review state.

[TFT444]

Both blockers resolved: Alembic chain fixed onto d8e4f6a1b2c3 and get_score now returns NO_SCAN_DATA instead of a false 100 when no scan exists.

[TFT444]

@parthrohit22 branch has conflict please solve them after it good to go

[parthrohit22]

CI was red on this branch's head after a dev merge picked up #277 (and #320) since the last push — validate_mapping_pack.py (this PR's own validator) correctly rejected #277's ten new AZ-NET-018..027 rules plus AZ-SECOPS-010, none of which carry the evidence-schema fields this PR requires, since neither existed yet when that requirement was written. Same root cause as the AZ-CMP-007 gap from #307 fixed earlier in this branch's history.

Fixed in the latest commit: filled in mapping_type/evidence_type/primary_source/rationale/review_status for all 44 combinations (11 rules × 4 framework files), following each entry's nearest sibling's exact convention in the same file (not_applicable for the CIS N/A-* control IDs, direct for AZ-SECOPS-010's real numbered CIS control, supporting elsewhere). Also cleaned up a merge artifact in CHANGELOG.md (a duplicate, malformed ## Unreleased header #277 had added above the file's existing well-formed section).

Verified: mapping-pack validator clean (was 55 errors), full backend suite (892 passed, 5 skipped — pre-existing/environment-only), alembic heads still resolves to exactly one head, ruff clean. All 20 CI checks are green on the current head.

[m-khan-97]

Parth, the mapping-pack work is strong: the Alembic chain is linear after d8e4f6a1b2c3, no-scan data is no longer reported as 100%, framework metadata and evidence semantics validate cleanly, and historical controls are captured with an integrity hash. I ran the mapping validator successfully and the focused non-route tests passed; the four local route setup errors were only because this host lacks prometheus_client, while authoritative CI is green.

I found one replay-consistency blocker in save_scan(). _scan_rule_outcomes.failed_rule_ids is stored inside compliance_mapping_snapshot, but the upsert preserves the entire first snapshot with COALESCE(scans.compliance_mapping_snapshot, EXCLUDED.compliance_mapping_snapshot). On a retry using the same scan_id, findings and status are deliberately replaced, yet the failed-rule set is not. If a transiently failed rule succeeds on the retry, the new findings are saved but compliance still reads the stale first-attempt rule as NOT_EVALUATED; the inverse is also possible. The comment calls the mapping snapshot immutable, which is correct for mapping provenance, but per-attempt execution outcomes are not immutable provenance.

Please preserve the framework snapshot while updating _scan_rule_outcomes to match the result being written, or store outcomes separately. Add a regression test that saves a scan ID with a failed rule, replays the same ID with that rule successful, and proves compliance no longer reports the stale NOT_EVALUATED state (and ideally the reverse direction as well). Once replayed findings and outcomes remain atomic, I will rereview.