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

[parthrohit22]

Partially addresses #302. It stays open on merge because independent review of the mapping pack is still outstanding (see "Limitations").

Problem and security impact

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

• Rules never evaluated scored as compliant. A rule with zero completed scans was reported as 100% compliant on every framework. PASS was derived only from "this rule_id is absent from today's failed-rule set", so an empty result because no scan existed looked the same as a scan that ran and found nothing.
• No distinction between strong and weak mappings. Every rule was force-mapped into all four frameworks (CIS, NIST, ISO 27001, SOC 2) with no rationale, evidence type or source. A rule giving only loose supporting evidence looked identical to a direct technical check. Frameworks that predate a rule's subject matter got mappings anyway (post-quantum rules mapped into 2013–2017 editions).
• Inconsistent control IDs. soc2.json and nist_csf.json recorded the same control_id under different control_name strings. nist_csf.json also mixed CSF 1.1 subcategories with six SP 800-53 codes that don't exist in CSF.
• No record of the mapping behind a historical scan. Scans didn't store which mapping-pack revision produced them, so an old report could be silently reinterpreted under whatever mapping file was deployed later.

How compliance status is derived now (contract_version 3)

• Status comes from evaluations. Each in-scope control's status is the rolled-up status of its rule's rule_evaluations rows for the scan (feat: persist PASS/FAIL/ERROR/NOT_APPLICABLE per rule per resource, fix compliance score #263/feat(engine): add rule evaluation coverage contract (#263) #321), via aggregate_status() (FAIL > ERROR > UNKNOWN > PASS > NOT_APPLICABLE). Findings are used only for severity, category and affected-resource detail.
• No evaluation rows means UNKNOWN, never PASS. A rule without any evaluation row for the scan is UNKNOWN. It is never a PASS inferred from the absence of a finding.
• Failed rules can only get stricter. A rule the engine recorded as failing to complete (_scan_rule_outcomes.failed_rule_ids) is forced to ERROR. That signal only ever makes a status stricter.
• UNKNOWN and ERROR stay in the denominator. They never count as a pass, so lost evidence lowers the score instead of shrinking the base.
• Excluded from the denominator: mapping_type not_applicable and organizational, plus any mapping whose review_status is not reviewed (reported as UNREVIEWED_MAPPING). Every shipped mapping is currently pending_review, so no unreviewed mapping can raise a score.
• Distinct empty states. NO_SCAN_DATA (no completed scan) returns null counts. It is distinct from a completed scan with no in-scope or no reviewed controls.

Implementation summary

• compliance/frameworks/*.json (all 6 packs):
  • Every control carries mapping_type, evidence_type, primary_source, rationale, owner, review_status and review_date.
  • Each pack has mapping_pack_version, status, source and published.
  • SOC 2 and NIST control_name inconsistencies are fixed, and the six SP 800-53 codes in nist_csf.json are remapped to CSF 1.1 subcategories.
  • AZ-PQC-* mappings in pre-PQC frameworks and the CIS N/A-* entries are not_applicable.
• api/models/finding.py:
  • get_compliance_score(framework, subscription_id) implements the model above.
  • get_score(subscription_id) returns NO_SCAN_DATA instead of a false 100.
  • Both are scoped by subscription; subscription_id is a real parameter supplied by the routes.
  • save_scan() writes the full framework mapping (controls plus a content hash) into scans.compliance_mapping_snapshot inside fix(core): harden scan durability and idempotency (#303) #325's lease-fenced UPDATE. Framework provenance is only filled in when absent. _scan_rule_outcomes is re-merged on every write and can never be nulled.
  • Provenance is reported as snapshot, snapshot_no_hash, snapshot_hash_mismatch or live_fallback_*.
• scanner/engine.py: records failed_rule_ids for any rule that raised or returned malformed data.
• alembic/versions/3a76ff935bf6_...: adds the nullable compliance_mapping_snapshot JSONB column, chained from d4a8c1e6b2f9 (fix(core): harden scan durability and idempotency (#303) #325's head). There is a single Alembic head.
• .github/scripts/validate_mapping_pack.py: mapping-pack semantics validator, run by CI.
• Routes: /api/score and /api/compliance/<framework> accept ?subscription_id=. They fall back to AZURE_SUBSCRIPTION_ID and return 400 on a malformed value.
• Frontend: api.js, ScoreGauge.jsx, FrameworkCards.jsx and Monitoring.jsx preserve null and status end to end instead of coercing to 0.
• Docs: docs/compliance-mapping-pack.md and docs/api-reference.md describe the evaluation-derived semantics and response contract.

Acceptance criteria (#302)

• PASS is emitted only from an explicit successful evaluation (rule_evaluations, feat: persist PASS/FAIL/ERROR/NOT_APPLICABLE per rule per resource, fix compliance score #263/feat(engine): add rule evaluation coverage contract (#263) #321).
• Framework name, edition, mapping-pack version and source are persisted with every scan, immutably across replay (full controls plus content hash).
• Current supported framework editions are documented; legacy packs are clearly versioned.
• N/A, organizational and unreviewed mappings are excluded from technical pass-rate denominators; UNKNOWN and ERROR are not.
• Every mapping has rationale, evidence type, primary source, owner and review date fields. owner and review_date stay null until someone reviews it.
• CI validates mapping semantics as a real, testable module.
• Public copy no longer claims full compliance or certification.
• A sample mapping set receives independent security/compliance review before release. This is a human step that a PR cannot self-certify.
• CIS entries are individually reviewed for direct vs. supporting/organizational, beyond the N/A-* reclassification.

Validation (on the current head, merged with dev at df98060)

Check	Result
alembic heads	single head 3a76ff935bf6
alembic upgrade head → downgrade -1 → upgrade head (PostgreSQL 16)	passes
python .github/scripts/validate_mapping_pack.py compliance/frameworks	valid
pytest tests/ --cov=api --cov=scanner --cov-fail-under=80 against PostgreSQL	1548 passed, 3 skipped, coverage 88.5%
ruff check . / ruff format --check .	clean
node frontend/src/utils/api.test.mjs / aiApi.test.mjs	36 passed / passed
frontend npm run lint / npm run build	pass
DCO	every commit signed off

Limitations and follow-up

• Independent review is not done. Every mapping's review_status is pending_review, with owner and review_date set to null. Because unreviewed mappings are excluded from the denominator, framework scores stay "no reviewed controls" until reviewers sign mappings off. That is deliberate: it is better than reporting a percentage built on unreviewed claims. CI rejects a reviewed entry that is missing an owner or date, so review can't be faked.
• CIS classification. Beyond the N/A-* fix, CIS direct classifications are still framework-level and need the per-control review above.
• ISO 27001 edition. Migrating the ISO 27001 pack to the 2022 edition is tracked separately in feat(compliance): migrate the ISO 27001 mapping pack from 2013 to ISO/IEC 27001:2022 #358.
• Stale marketing content. website/content.js still shows "NIST": "AC-17" for AZ-KV-002. It is static marketing content with no dependency on the framework files, so it is out of scope here.

No secrets or sensitive infrastructure data

Confirmed: this PR adds no credentials, tokens, connection strings, IPs or hostnames.

[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.

[parthrohit22]

@m-khan-97 addressed your Sep 7 blocker on 57aa6f2. The head no longer derives compliance status from findings; get_compliance_score() is evaluation-derived again, per #263/#321:

• Each in-scope control's status is the rolled-up status of its rule's rule_evaluations rows for the scan, via aggregate_status() (FAIL > ERROR > UNKNOWN > PASS > NOT_APPLICABLE).
• A control whose rule has no evaluation row for the scan is UNKNOWN, never a PASS inferred from the absence of a finding. The regression test_get_compliance_score_no_evaluation_rows_is_unknown_not_pass is restored (it had been renamed and flipped to PASS).
• _scan_rule_outcomes.failed_rule_ids is kept only as a one-way stricter signal: a rule the engine recorded as failing to complete is forced to ERROR. It never loosens a status and never shrinks the denominator.
• Denominator excludes only mapping_type not_applicable/organizational. UNKNOWN and ERROR stay in it and never count as a pass, so lost/missing evidence lowers the score rather than shrinking the base — the NOT_EVALUATED-excluded-from-denominator behaviour you flagged is gone.
• Findings are used only for severity/category/affected-resource detail.

Subscription scoping, NO_SCAN_DATA, NO_IN_SCOPE_CONTROLS, the mapping-pack snapshot/provenance/content-hash layer, and the per-control evidence-schema fields are unchanged. New coverage in test_compliance_scoring.py: persisted UNKNOWN/ERROR reported not promoted, mixed PASS/UNKNOWN, worst-resource-status-wins, ERROR-stays-in-denominator. evaluation_basis, docs/api-reference.md and docs/compliance-mapping-pack.md updated to describe the evaluation-derived semantics.

Also merged current dev: resolved the nist_csf.json conflict (kept this branch's enriched AZ-DB/COSMOS/CACHE entries over #278's compact duplicates; repointed AZ-DB-007 from the stray ISO id A.12.4.1 to NIST CSF PR.PT-1). alembic heads → single head 3a76ff935bf6 (chained after #321's 3f59f83a5253). Full backend suite 1031 passed / 7 skipped, ruff clean, mapping-pack validator clean, frontend api tests 34 passed. All checks green.

@TFT444 @ritiksah141 re-requesting review since the scoring path changed materially from what you approved.

[ritiksah141]

All addressed, so approving it.

[parthrohit22]

@m-khan-97 please re-review this, I think it looks ready. Thank you.

[TFT444]

@parthrohit22 three issues remain:

1. _scan_rule_outcomes null-merge bug (finding.py, ON CONFLICT block): if a retry has zero failed rules, EXCLUDED.compliance_mapping_snapshot -> '_scan_rule_outcomes' is NULL and the || jsonb_build_object(...) overwrites a prior non-null value with null. Use a CASE/WHEN guard so the key is only merged when non-null.

2. N/A entries missing required fields: the new CI validator (validate_mapping_pack.py) requires mapping_type, evidence_type, primary_source, rationale, and review_status on every control. The N/A entries (AZ-DB-005, AZ-FUNC-001, etc.) only carry control_id, control_name, description. The validator will fail on merge as-is.

3. get_score() subscription_id not fixed: getattr(self, "subscription_id", None) always returns None because DatabaseManager has no such attribute. The subscription-scoped query branch never executes. Pass it as a parameter the same way get_compliance_score() now does.

[parthrohit22]

@TFT444 thanks — pushed a3e6dc2. Two of the three are fixed; on the third I think the finding doesn't reproduce, evidence below.

1. _scan_rule_outcomes null-merge — fixed

You're right that the merge could write a JSON null, and the root cause was upstream of the SQL: save_scan() only set the key when a rule had failed, so a clean retry sent a snapshot with no _scan_rule_outcomes at all, EXCLUDED.compliance_mapping_snapshot -> '_scan_rule_outcomes' was SQL NULL, and jsonb_build_object stamped null over the stored value.

I fixed it at both ends rather than only guarding the SQL, because a CASE/WHEN guard alone would have re-introduced the bug @m-khan-97 blocked on in the Aug 31 round — if the key is only merged when non-null, a retry that clears a previously failed rule can never clear it, and the rule stays ERROR forever.

• save_scan() now always writes the key, empty list included. "No rule failed on this attempt" is a real per-attempt result, not missing data.
• The re-merge COALESCEs through the stored value to an explicit empty list, so no write path — including one that predates this change — can null it out:

'_scan_rule_outcomes',
COALESCE(
    EXCLUDED.compliance_mapping_snapshot -> '_scan_rule_outcomes',
    scans.compliance_mapping_snapshot -> '_scan_rule_outcomes',
    '{"failed_rule_ids": []}'::jsonb
)

Per-attempt refresh semantics are unchanged: our own writer always supplies the key, so the first COALESCE arm always wins and a retry still replaces the prior attempt's outcomes. The fallback arms only matter for a snapshot written without the key.

Tests updated to the new contract and extended to pin the guard: test_save_scan_writes_empty_scan_rule_outcomes_when_nothing_failed (renamed from ..._omits_..., which asserted the behaviour that caused this) and test_save_scan_upsert_refreshes_scan_rule_outcomes_on_every_write, which now asserts both fallback arms are present.

2. N/A entries missing required fields — doesn't reproduce on this head

I couldn't reproduce this one. The N/A entries you named do carry all five fields on the current head:

$ python3 -c "import json; print(sorted(json.load(open('compliance/frameworks/cis_azure_benchmark.json'))['controls']['AZ-DB-005'].keys()))"
['control_id', 'control_name', 'description', 'evidence_type', 'mapping_type',
 'owner', 'primary_source', 'rationale', 'review_date', 'review_status']

Same for AZ-FUNC-001, AZ-DB-006, AZ-DB-007 and AZ-CACHE-001, across all four non-PQC packs.

Because the concern was specifically "will fail on merge", I ran the validator against GitHub's own computed merge ref rather than the branch, so the check includes the merge with current dev:

$ git fetch upstream refs/pull/310/merge && git archive refs/remotes/pr310merge \
    compliance/frameworks .github/scripts/validate_mapping_pack.py | tar -x -C /tmp/m
$ cd /tmp/m && python3 .github/scripts/validate_mapping_pack.py compliance/frameworks
=== Validating compliance mapping-pack semantics in compliance/frameworks ===
All compliance mappings carry valid mapping-pack semantics.

The Rule & Compliance Validation job — which is what runs validate_mapping_pack.py in CI — is also green on a3e6dc2. If you were looking at dev's copies of those entries rather than this branch's, that would explain the difference: the metadata is added by this PR. Happy to be shown otherwise if you're seeing a failure I'm not.

3. get_score() subscription_id — fixed

Correct, and worse than not-fixed: getattr(self, "subscription_id", None) meant the scoped branch was unreachable in production, so it read as handled while every real call went down the unfiltered path. subscription_id is now a real parameter supplied by the route, exactly as get_compliance_score() does it:

def get_score(self, subscription_id: Optional[str] = None) -> Dict[str, Any]:

GET /api/score now takes ?subscription_id=, falls back to the deployment's AZURE_SUBSCRIPTION_ID, and returns 400 on a malformed value rather than silently falling back to an unscoped score.

Regression coverage: test_route_passes_subscription_id_through_to_the_model, test_route_falls_back_to_the_configured_default_subscription, test_route_rejects_a_malformed_subscription_id, plus test_get_score_ignores_a_stray_subscription_id_attribute, which sets the old instance attribute and proves it no longer scopes the query — so the dead-code path can't come back unnoticed.

I also updated docs/api-reference.md, which listed "Query parameters: none" for both /api/score and /api/compliance/<framework> — the latter was already stale, since that endpoint has accepted subscription_id since the earlier round.

Validation

1044 passed locally (4 pre-existing failures in test_devops_client.py / test_observability.py are environment-only — no local Postgres and no network — and fail identically on the unmodified head). ruff check clean. All 21 checks green on a3e6dc2.

[TFT444]

All three blockers from the Sep 12 review are resolved:

1. _scan_rule_outcomes null-merge now uses COALESCE(EXCLUDED.compliance_mapping_snapshot -> '_scan_rule_outcomes', scans.compliance_mapping_snapshot -> '_scan_rule_outcomes') — a zero-findings retry can no longer overwrite a prior non-null value.
2. N/A entries carry the full required field set (mapping_type, evidence_type, primary_source, rationale, review_status). The CI validator will pass.
3. get_score() accepts subscription_id as a parameter and applies AND subscription_id = %s correctly — no more getattr(self, ...).

All earlier blockers (Alembic chain, CIS direct misclassifications, NO_SCAN_DATA false-100, COALESCE replay atomicity) were resolved in prior rounds. CI is green. Approving.

[parthrohit22]

Merged current dev in; the branch is mergeable again. Three conflicts, one of which was substantive:

1. CHANGELOG.md — additive, kept both sides.

2. frontend/src/utils/api.test.mjs — dev rewrote loadApiModule() for its memory-only bearer-token work while this branch had extended the same function. Kept both: loadApiModule now takes dev's storage option and the function shorthand this branch's tests use, and exports normalizeScore/normalizeComplianceFramework alongside dev's store. Both test sets kept, 36/36 pass.

3. compliance/frameworks/*.json — the one worth reviewing. The 17 rules dev added since the last sync (AZ-CMP-005, AZ-CMP-006, AZ-AKS-007..021) arrived carrying only control_id/control_name/description, so .github/scripts/validate_mapping_pack.py failed with 340 errors — exactly the guard this PR adds, catching the first mapping pack to land without evidence metadata.

Annotated all 68 entries (17 rules × 4 frameworks) following each framework's existing template:

• CIS: not_applicable + evidence_type: not_applicable for the 16 synthetic N/A-* ids, and direct for AZ-AKS-010 only, which maps to a real recommendation (CIS 2.1.8, Defender for Containers).
• ISO 27001 / NIST CSF / SOC 2: supporting, consistent with every other rule in those packs.
• Every one is review_status: pending_review, owner: null, review_date: null, so they are listed but excluded from the scored denominator until someone actually reviews them. No new rule silently inflates a score.
• Mapping packs bumped 1.0.0 → 1.1.0 with a new mapping_pack_published, since the pack content changed.

Verification on the merged head: validate_mapping_pack.py clean, pytest tests/ 1330 passed / 9 skipped, node frontend/src/utils/api.test.mjs 36/36, and alembic is still a single linear head at 3a76ff935bf6.

@m-khan-97 on your 7 Sep blocker — the evaluation-derived contract is in place on this head and survived the merge: get_compliance_score() reads rule_evaluations for the scan and rolls them up through aggregate_status(); a rule with no evaluation row is UNKNOWN, never a PASS inferred from the absence of a finding. tests/test_compliance_scoring.py covers the completed-scan-with-no-evaluation-rows case and the mixed PASS/UNKNOWN case. Re-requesting your review.

@TFT444 @ritiksah141 re-requesting as well, since the merge touched the mapping packs your earlier findings were about.

[TFT444]

All blockers from my Sep 12 review are resolved as confirmed in my Sep 19 note. The branch is currently conflicting against dev. Please rebase onto current dev and push. Also pinging @m-khan-97 to re-review the latest commit as their Sep 7 CHANGES_REQUESTED was addressed in later pushes but they have not formally signed off on the current head.

[parthrohit22]

Merged current dev in (0ed37bd). It's mergeable again. #325 landed in between, so this wasn't just a pointer fix:

• save_scan: fix(core): harden scan durability and idempotency (#303) #325 replaced the INSERT ... ON CONFLICT with a lease-fenced UPDATE. The compliance_mapping_snapshot write now happens inside that fenced UPDATE, with the same semantics as before. Framework provenance is only filled in when absent, and _scan_rule_outcomes is re-merged on every write and can never be nulled. A worker that lost its lease can't touch it either.
• Migration: 3a76ff935bf6 now chains from d4a8c1e6b2f9 (fix(core): harden scan durability and idempotency (#303) #325's head), and alembic heads returns a single head. Upgrade and downgrade are both checked on PostgreSQL 16.
• Mapping packs: kept this branch's reviewed entries and added feat(scanner): add rule AZ-STOR-010 storage account missing private endpoint #327's AZ-STOR-010 with the full semantics fields: CIS not applicable, ISO A.13.1.3, NIST PR.AC-5, SOC 2 CC6.6, all pending_review. validate_mapping_pack.py passes.
• Tests: the save_scan tests now use the fenced signature. Test: Add missing coverage for DatabaseManager, API routes, Sentinel ingest and RAG pipeline #174's new get_score/compliance-route tests follow this branch's contracts. The admission migration tests now resolve the head from the revision graph instead of pinning d4a8c1e6b2f9, so the next migration won't break them.

Full suite against Postgres: 1487 passed, 3 skipped.

@m-khan-97 your 7 Sep blocker was addressed in 57aa6f2, but the review was never re-run. PASS now requires an explicit evaluation row. A completed scan with no evaluation row reports UNKNOWN (test_direct_control_with_no_evaluation_row_is_unknown_not_pass). Persisted UNKNOWN/ERROR are reported, not promoted (test_persisted_unknown_and_error_evaluations_are_reported_not_promoted). Mixed coverage is covered too, and the response now declares contract_version: "3". Could you take another look?

@TFT444 @ritiksah141 your approvals were dismissed by the merge push. Re-requesting in case you want to glance at the save_scan resolution.

[parthrohit22]

Merged current dev (81912c9): #344 and #359. It's mergeable again. The only conflict was CHANGELOG.md, and I kept both sides. #359's AI routes use get_scan / get_latest_completed_scan / get_findings, which this branch doesn't change.

Re-verified on the merged head against PostgreSQL 16:

• a single Alembic head (3a76ff935bf6), and upgrade → downgrade → upgrade passes
• the mapping-pack validator is clean
• pytest tests/ passes: 1548 passed, 3 skipped, coverage 88.5%
• ruff is clean
• the frontend API tests (36) and the aiApi tests pass, and lint and build pass

I also rewrote the PR description. It still described the old NOT_EVALUATED-excluded-from-denominator model, which is the thing @m-khan-97 blocked on and which hasn't been in the code since 57aa6f2. It now describes contract v3 as it is implemented:

• Status comes from rule_evaluations.
• A rule with no evaluation row is UNKNOWN, never PASS.
• Engine failures force ERROR.
• UNKNOWN and ERROR stay in the denominator.
• Only not_applicable, organizational and unreviewed mappings are excluded.

@m-khan-97 your 7 Sep review is the only change request still open. The fix has been on the branch since 57aa6f2 (tests test_direct_control_with_no_evaluation_row_is_unknown_not_pass and test_persisted_unknown_and_error_evaluations_are_reported_not_promoted). Could you re-review the current head?

@TFT444 @ritiksah141 your approvals were dismissed by earlier merge pushes. Nothing in the logic changed in this merge.

[TFT444]

Re-reviewed current head 81912c9. The commits since my Sep 19 approval (9e918f0) are all chore: merge upstream dev merges with no substantive changes to the compliance scoring logic, API routes, or tests. My approval of 9e918f0 stands: the three blockers I raised on Sep 12 are all resolved, CI is green, and the scoring is now evidence-derived and non-certifying as required.

The admission-test head pin (get_current_head() approach from this PR) will also unblock #352 once that migration's down_revision is fixed to a single parent. Merge order note: whichever of #310 or #352 lands second will need to repoint its down_revision to the other's revision ID. Worth coordinating with @TFT444.