Implementation Plan

Overview

Five compliance code paths share the root cause "key by compliance_uploads.id instead of by compliance_uploads.report_date": GET /trends, GET /top-recurring, GET /category-trend, GET /summary, and the compliance_snapshots block inside persistUpload(). All fixes are contained to backend/routes/compliance.js and require no schema migration, no new column, and no frontend change.

The plan follows the bugfix workflow's exploratory methodology: a single property-based test file (backend/__tests__/compliance-duplicate-chart-entries.property.test.js) is written before any fix, with one test case per affected site demonstrating the bug condition, plus preservation cases observed on the unfixed code. Each fix is then implemented as its own task and verified by re-running the matching test case from the exploration suite. The plan ends with a regression checkpoint that re-runs the full preservation suite and the backend test suite.

Task Dependency Graph

{
  "waves": [
    { "id": 0, "tasks": ["1", "2"] },
    { "id": 1, "tasks": ["3.1", "4.1", "5.1", "6.1", "7.1"] },
    { "id": 2, "tasks": ["3.2", "4.2", "5.2", "6.2", "7.2"] },
    { "id": 3, "tasks": ["8"] },
    { "id": 4, "tasks": ["9"] }
  ]
}

Wave 0 establishes the test baseline: task 1 documents Property 1 (bug condition) failures on the unfixed code and task 2 captures Property 2 (preservation) baseline outputs. Wave 1 implements the five independent fixes in backend/routes/compliance.js (no inter-fix dependencies — each touches a different SQL statement). Wave 2 verifies each fix's slice of Property 1 now passes. Wave 3 re-runs the full Property 2 suite to confirm no regressions across the five sites. Wave 4 is the final checkpoint that runs the entire backend test suite.

Tasks

1. Write bug condition exploration property test
- Property 1: Bug Condition - Multi-Vertical Date Aggregation Across Five Compliance Sites
- CRITICAL: This test MUST FAIL on unfixed code — failure confirms the bug exists across all five sites
- DO NOT attempt to fix the test or the code when it fails
- NOTE: This test encodes the expected behavior — it will validate the fixes when it passes after implementation
- GOAL: Surface counterexamples that demonstrate the bug exists for each of the five affected code paths (/trends, /top-recurring, /category-trend, /summary, persistUpload() snapshot block)
- Scoped PBT Approach: Scope the property to the canonical bug-condition fixture (fixture_multi_vertical_single_date from design.md) — three uploads for 2025-05-11, one each for NTS_AEO, SDIT_CISO, TSI, with distinct counts and matching items per upload — this reproduces the original GitLab #12 scenario deterministically
- Bug Condition (from design.md): EXISTS report_date d WHERE COUNT(compliance_uploads WHERE report_date = d) > 1
- Create backend/__tests__/compliance-duplicate-chart-entries.property.test.js using fast-check and the existing pg pool mock pattern from vcl-compliance-reporting.property.test.js
- Test case 1.A — /trends duplicate-date counterexample: seed three uploads for 2025-05-11 (verticals NTS_AEO/SDIT_CISO/TSI), call GET /trends, assert response.trends.filter(t => t.report_date === '2025-05-11').length === 1 AND new_count for that date equals the sum of the three uploads' new_count values (likewise recurring_count, resolved_count, total_active, and per-team counts)
- Test case 1.B — /top-recurring duplicate-bar counterexample: same fixture, call GET /top-recurring, assert exactly one waterfall entry per unique report_date AND the running invariant entry[i].end === entry[i].start + entry[i].new_count + entry[i].recurring_count - entry[i].resolved_count holds for every i AND entry[i].start === entry[i-1].end for adjacent entries (with entry[0].start === 0)
- Test case 1.C — /category-trend duplicate (date, category) counterexample: same fixture plus items tagged with two categories (Patching and Configuration), call GET /category-trend, assert response.categoryTrend.filter(c => c.report_date === '2025-05-11' && c.category === 'Patching').length === 1 AND each entry's count equals the total compliance_items for that category across every upload sharing the date
- Test case 1.D — /summary sibling-disclosure counterexample: same fixture (2025-05-11 is the latest date), call GET /summary, assert either (a) entries is the merged view of all three uploads OR (b) response.multi_vertical_uploads is a non-empty array with length === 2 listing the other two uploads' ids and verticals
- Test case 1.E — persistUpload() cross-vertical contamination counterexample: pre-populate compliance_items with disjoint sets for two verticals (e.g., NTS_AEO has 100 active items, SDIT_CISO has 50 active items), call persistUpload() for a fresh SDIT_CISO upload, read back the compliance_snapshots row for the current month with vertical = 'SDIT_CISO', assert total_devices reflects only SDIT_CISO items and is not inflated by NTS_AEO items
- Wrap each test case in fast-check fc.assert against arbScenario from design.md (uploads with possibly colliding report_date, items referencing those uploads) so PBT also exercises larger random fixtures beyond the canonical 3-upload case
- Add fixture builders in the test file matching the design's fixture_multi_vertical_single_date and fixture_cross_vertical_items
- Run test on UNFIXED code: npm run test:backend -- compliance-duplicate-chart-entries.property.test.js
- EXPECTED OUTCOME: All five test cases FAIL (this is correct — it proves each manifestation of the bug exists)
- Document the counterexamples found (e.g., /trends returns 3 entries for 2025-05-11 instead of 1, /summary returns one upload's summary_json and silently drops the other two, compliance_snapshots.total_devices for SDIT_CISO equals 150 instead of 50)
- Mark task complete when test is written, run, and the failures for all five test cases are documented
- Requirements: 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.8, 1.9, 1.10, 1.11
2. Write preservation property tests (BEFORE implementing fix)
- Property 2: Preservation - Single-Upload-Per-Date Behavior Unchanged Across Five Sites
- IMPORTANT: Follow observation-first methodology — capture pre-fix outputs and assert equality post-fix
- Bug Condition negation (from design.md): FORALL report_date d, COUNT(compliance_uploads WHERE report_date = d) <= 1
- Add preservation test cases to backend/__tests__/compliance-duplicate-chart-entries.property.test.js
- Observe baseline behavior on UNFIXED code using the design's preservation fixtures and capture exact response bodies (snapshot-test style)
- Test case 2.A — Empty-state preservation (fixture_empty): no compliance_uploads, no compliance_items. Observe GET /trends returns { trends: [] }, GET /top-recurring returns { waterfall: [] }, GET /category-trend returns { categoryTrend: [] }, GET /summary returns { entries: [], overall_scores: {}, upload: null }. Capture and assert these exact shapes
- Test case 2.B — Single AEO-legacy-upload preservation (fixture_single_upload_aeo_legacy): one upload with vertical IS NULL, report_date = '2025-04-01', ~20 items across the four teams. Observe responses from all four read endpoints, capture them, and assert byte-for-byte equality
- Test case 2.C — Multiple single-upload-per-date preservation (fixture_single_upload_per_date): five uploads on five distinct dates with varied vertical values. Observe responses from all four read endpoints and assert equality
- Test case 2.D — /summary team query parameter preservation: with the latest upload present, assert ?team=STEAM filters entries server-side AND ?team=OTHER (non-ALLOWED_TEAMS) returns HTTP 400. Capture both responses
- Test case 2.E — persistUpload() single-vertical-month preservation (fixture_cross_vertical_items reduced to one vertical): pre-populate compliance_items with rows from a single vertical only, run persistUpload() for that vertical, capture the resulting compliance_snapshots rows
- Test case 2.F — persistUpload() snapshot error-path preservation: force a snapshot query failure (mock pool.query to reject on the snapshot statement only), assert the upload still commits and the error is logged but not surfaced (HTTP 200/201, no error response)
- Property-based extension — Cross-endpoint preservation: use fast-check arbScenario constrained to scenarios where every report_date has exactly one upload row. Assert that for every generated scenario, all four endpoint responses on UNFIXED code match the captured-baseline shape and field-level equality holds (this generator covers the design's fixture_pbt_generators.arbScenario restricted to the non-bug-condition input space)
- Run tests on UNFIXED code: npm run test:backend -- compliance-duplicate-chart-entries.property.test.js
- EXPECTED OUTCOME: All preservation test cases PASS (this confirms the baseline behavior to preserve)
- Mark task complete when tests are written, run, and passing on unfixed code
- Requirements: 3.1, 3.2, 3.3, 3.4, 3.5, 3.6, 3.7, 3.8, 3.9, 3.10
3. Fix /trends — aggregate uploads and team counts by report_date
- 3.1 Rewrite the /trends upload and team queries to group by report_date
  - In backend/routes/compliance.js router.get('/trends', ...) (around line 768), replace the compliance_uploads query with the GROUP BY report_date SQL from design.md Fix 1, summing new_count, recurring_count, resolved_count, and (new_count + recurring_count) AS total_active
  - Replace the per-team compliance_items query with the JOIN compliance_uploads + GROUP BY cu.report_date, ci.team form from design.md Fix 1
  - Change the teamMap keyed lookup from teamMap[u.id] to teamMap[u.report_date] and rebuild trends from the per-date upload rows
  - Bug_Condition: isBugCondition(uploads) where two or more compliance_uploads rows share a report_date
  - Expected_Behavior: GET /trends returns one entry per unique report_date with summed count fields and aggregated per-team counts (Property 1 from design)
  - Preservation: Single-upload-per-date dates produce identical responses; empty-data response remains { trends: [] }; chart components require no changes
  - Requirements: 2.1, 2.2, 2.3, 3.1, 3.2, 3.3
- 3.2 Verify the /trends portion of bug condition exploration test now passes
  - Property 1: Expected Behavior - /trends Returns One Entry Per Unique report_date
  - IMPORTANT: Re-run the SAME test case 1.A from task 1 — do NOT write a new test
  - Run npm run test:backend -- compliance-duplicate-chart-entries.property.test.js -t "/trends"
  - EXPECTED OUTCOME: Test case 1.A PASSES (confirms /trends bug is fixed)
  - Requirements: Property 1 (Validates 2.1, 2.2, 2.3) from design
4. Fix /top-recurring — aggregate uploads by report_date before passing to computeWaterfall()
- 4.1 Rewrite the /top-recurring upload query to group by report_date
  - In backend/routes/compliance.js router.get('/top-recurring', ...) (around line 818), replace the query with the GROUP BY report_date SQL from design.md Fix 2, summing new_count, recurring_count, resolved_count
  - Leave computeWaterfall() unchanged — it already advances start correctly when fed one row per date; the fix is purely in the SQL
  - Bug_Condition: isBugCondition(uploads) where two or more compliance_uploads rows share a report_date
  - Expected_Behavior: GET /top-recurring returns one waterfall entry per unique report_date with summed deltas; running invariant entry[i].end === entry[i].start + entry[i].new_count + entry[i].recurring_count - entry[i].resolved_count holds and entry[i].start === entry[i-1].end for adjacent entries (Property 2 from design)
  - Preservation: Single-upload-per-date waterfall is unchanged; empty-data response remains { waterfall: [] }
  - Requirements: 2.4, 2.5, 3.4
- 4.2 Verify the /top-recurring portion of bug condition exploration test now passes
  - Property 1: Expected Behavior - /top-recurring Has One Bar Per Unique report_date With Correct Running Totals
  - IMPORTANT: Re-run the SAME test case 1.B from task 1 — do NOT write a new test
  - Run npm run test:backend -- compliance-duplicate-chart-entries.property.test.js -t "/top-recurring"
  - EXPECTED OUTCOME: Test case 1.B PASSES (confirms /top-recurring bug is fixed and the running invariant holds)
  - Requirements: Property 2 (Validates 2.4, 2.5) from design
5. Fix /category-trend — drop cu.id from GROUP BY
- 5.1 Rewrite the /category-trend query to group by (report_date, category) only
  - In backend/routes/compliance.js router.get('/category-trend', ...) (around line 838), replace the query with the SQL from design.md Fix 3 — remove cu.id from the GROUP BY so grouping is by (cu.report_date, COALESCE(ci.category, 'Unknown')) only
  - Leave the response shape { categoryTrend: Array<{ report_date, category, count }> } unchanged
  - Bug_Condition: isBugCondition(uploads) where two or more compliance_uploads rows share a report_date
  - Expected_Behavior: GET /category-trend returns one row per unique (report_date, category) pair with count equal to the total compliance_items for that category across every upload sharing the date (Property 3 from design)
  - Preservation: Single-upload-per-date rows are unchanged; empty-data response remains { categoryTrend: [] }; total-conservation property holds across all dates (Property 6 from design)
  - Requirements: 2.6, 2.7, 3.5
- 5.2 Verify the /category-trend portion of bug condition exploration test now passes
  - Property 1: Expected Behavior - /category-trend Returns One Row Per (date, category)
  - IMPORTANT: Re-run the SAME test case 1.C from task 1 — do NOT write a new test
  - Run npm run test:backend -- compliance-duplicate-chart-entries.property.test.js -t "/category-trend"
  - EXPECTED OUTCOME: Test case 1.C PASSES (confirms /category-trend bug is fixed)
  - Requirements: Property 3 (Validates 2.6, 2.7) from design
6. Fix /summary — disclose sibling uploads for the latest date
- 6.1 Add sibling-upload disclosure to the /summary response
  - In backend/routes/compliance.js router.get('/summary', ...) (around line 495), keep the existing vertical IS NULL → vertical = 'NTS_AEO' fallback for selecting the primary upload's summary_json (preserves legacy single-upload behavior per requirement 3.6)
  - After resolving latestUpload, run the second query from design.md Fix 4 to find sibling uploads sharing the same report_date: SELECT id, vertical, uploaded_at FROM compliance_uploads WHERE report_date = $1 AND id != $2 ORDER BY id ASC
  - Add multi_vertical_uploads to the response, populated with siblings.map(s => ({ id, vertical, uploaded_at })); the field is [] when no siblings exist
  - Do not change the team query parameter handling, the ALLOWED_TEAMS HTTP 400 response, or the entries/overall_scores/upload shape
  - Bug_Condition: isBugCondition(uploads) where two or more compliance_uploads rows share the latest report_date
  - Expected_Behavior: GET /summary either merges sibling uploads' entries OR exposes a non-empty multi_vertical_uploads array identifying the other uploads for the same report_date; sibling uploads are never silently dropped (Property 4 from design)
  - Preservation: Single-upload-per-date /summary shape is unchanged (the vertical IS NULL → vertical = 'NTS_AEO' fallback still runs); team query parameter still filters entries and rejects non-ALLOWED_TEAMS with HTTP 400; empty-data response remains { entries: [], overall_scores: {}, upload: null }
  - Requirements: 2.8, 2.9, 3.6, 3.7
- 6.2 Verify the /summary portion of bug condition exploration test now passes
  - Property 1: Expected Behavior - /summary Does Not Silently Drop Sibling Uploads
  - IMPORTANT: Re-run the SAME test case 1.D from task 1 — do NOT write a new test
  - Run npm run test:backend -- compliance-duplicate-chart-entries.property.test.js -t "/summary"
  - EXPECTED OUTCOME: Test case 1.D PASSES (confirms /summary discloses sibling uploads via multi_vertical_uploads)
  - Requirements: Property 4 (Validates 2.8, 2.9) from design
7. Fix persistUpload() snapshot block — filter and group by vertical
- 7.1 Rewrite the verticalStats query to filter by the upload's vertical
  - In backend/routes/compliance.js persistUpload() (lines 81–192), at the verticalStats query around line 157, capture the upload's vertical from the row returned by the RETURNING id insert (or accept it as a persistUpload() parameter)
  - Replace the verticalStats query with the SQL from design.md Fix 5: filter WHERE team IS NOT NULL AND vertical IS NOT DISTINCT FROM $1 and group by (vertical, team). The IS NOT DISTINCT FROM operator handles the legacy vertical IS NULL case so AEO-only uploads keep their previous semantics
  - Leave the existing INSERT ... ON CONFLICT (snapshot_month, vertical) DO UPDATE mapping as-is so compliance_snapshots consumers (/vcl/stats) continue to read the same column shape; only the underlying counts change
  - Keep snapshot creation wrapped in the existing try/catch so a snapshot failure is logged and does not fail the upload commit
  - Bug_Condition: isBugCondition for persistUpload() is compliance_items containing rows for verticals other than the upload's vertical — the unfiltered query inflates total_devices/compliant/non_compliant
  - Expected_Behavior: compliance_snapshots rows written by persistUpload() have total_devices, compliant, non_compliant derived only from compliance_items rows belonging to the snapshotted vertical; no item from another vertical contributes (Property 5 from design)
  - Preservation: Single-vertical months produce identical snapshot rows; the legacy vertical IS NULL AEO-only path is unchanged via IS NOT DISTINCT FROM; the snapshot try/catch error path is unchanged
  - Requirements: 2.10, 2.11, 3.8, 3.9
- 7.2 Verify the persistUpload() portion of bug condition exploration test now passes
  - Property 1: Expected Behavior - persistUpload() Snapshot Reflects Only the Snapshotted Vertical
  - IMPORTANT: Re-run the SAME test case 1.E from task 1 — do NOT write a new test
  - Run npm run test:backend -- compliance-duplicate-chart-entries.property.test.js -t "persistUpload"
  - EXPECTED OUTCOME: Test case 1.E PASSES (confirms snapshot rows are filtered to the snapshotted vertical only)
  - Requirements: Property 5 (Validates 2.10, 2.11) from design
8. Verify preservation tests still pass after all five fixes
- Property 2: Preservation - Single-Upload-Per-Date Behavior Unchanged
- IMPORTANT: Re-run the SAME tests from task 2 — do NOT write new tests
- Run preservation property tests from task 2: npm run test:backend -- compliance-duplicate-chart-entries.property.test.js -t "Preservation"
- Confirm all six preservation test cases (2.A–2.F) and the cross-endpoint property-based preservation extension still pass
- EXPECTED OUTCOME: Tests PASS (confirms no regressions across the four read endpoints, the /summary team filter, the persistUpload() single-vertical-month path, and the snapshot error-path behavior)
- Requirements: 3.1, 3.2, 3.3, 3.4, 3.5, 3.6, 3.7, 3.8, 3.9, 3.10
9. Checkpoint — Ensure all tests pass
- Run the full backend test suite: npm run test:backend
- Confirm compliance-duplicate-chart-entries.property.test.js passes end-to-end (both Property 1 expected-behavior cases and Property 2 preservation cases)
- Confirm pre-existing tests (vcl-compliance-reporting.property.test.js, vcl-aggregated-burndown.property.test.js, vcl-aggregated-burndown.test.js, vcl-compliance-reporting.test.js, fp-submissions-cleanup.test.js, etc.) still pass — none of these should be affected since the fix is contained to read queries and one snapshot write query in compliance.js
- Spot-check the integration scenarios from design.md "Integration Tests": upload three xlsx files for the same report_date via POST /preview + POST /commit, then call /trends, /top-recurring, /category-trend, /summary and verify aggregated/disclosed responses; call /vcl/stats and verify per-vertical compliance_pct is correct
- Ensure all tests pass, ask the user if questions arise

Notes

All five fixes are contained to backend/routes/compliance.js. No database migration, no new column, no frontend change.
The Property 1 / Property 2 task numbering follows the bugfix workflow convention so the IDE hover-status indicator can track exploration tests against expected-behavior verification. Task 1 is the single Property 1 source; tasks 3.2 / 4.2 / 5.2 / 6.2 / 7.2 each re-run the relevant slice of Property 1 (NOT new tests) to confirm the matching fix lands correctly. Task 2 is the single Property 2 source; task 8 re-runs Property 2 in full to confirm no regressions.
The implementation tasks (3 through 7) are independent at the SQL level. Each can be reviewed and merged without waiting on the others, as long as task 1 and task 2 have run on the unfixed code first.
The _Bug_Condition, _Expected_Behavior, and _Preservation annotations on each fix sub-task reference the formal pseudocode in design.md Glossary and Bug Details sections.
_Requirements: X.Y_ annotations cite clauses in bugfix.md Bug Analysis.

22 KiB Raw Blame History Unescape Escape

Implementation Plan

Overview

Task Dependency Graph

Tasks

Notes

22 KiB

Raw Blame History