docs: append task 2 review fix report

2026-09-22 18:01:04 -03:00
parent e35417f044
commit 041257d254
1 changed files with 58 additions and 0 deletions
@@ -76,3 +76,61 @@ Implementation commit: `d729710` (`feat: add bounded operational view models`).

 - `BlockedProduct` has no persisted installation foreign key. Blocker list/detail identity therefore uses the lowest-ID installation for the tenant as the representative context; agent and installation filters check tenant installation membership without duplicating blocker rows.
 - The existing non-failing pytest environment warnings remain as noted above.
+
+## Task 2 Review Fix Report
+
+### Findings Fixed
+
+- Added nullable `blocked_products.installation_id` with a foreign key to `installations.id` and index in Alembic revision `0018_blocker_installation_identity`; updated the SQLAlchemy model.
+- Updated blocker upsert and authenticated job completion to persist the authenticated Agent installation. Agent-facing blocker list/reprocess/export paths and control-panel agent detail routes now use exact installation ownership. Legacy null-owned rows remain tenant-level and render without fabricated server context.
+- Replaced dashboard installation/blocker materialization with SQL health counts, snapshot totals, open-row count, distinct tenant/product count, and blocker-field breakdown. Health, blocker, and version attention candidates are individually bounded before Python merge/truncation.
+- Replaced Python health filtering/slicing with a portable SQL `CASE` expression using the supplied `now`, failure/job fields, heartbeat cutoff, SQL count, order, offset, and limit.
+- Worst-coverage ordering now uses the same derived complete/total percentage, clamped and rounded consistently with presentation.
+
+### Review-Fix RED Evidence
+
+Command after adding the review regressions:
+
+```text
+python -m pytest central/tests/test_operational_view_models.py central/tests/test_jobs.py -q
+```
+
+Result: the new regressions failed as expected: exact ownership was unsupported, legacy blockers received the fabricated representative installation, dashboard SQL `DISTINCT` aggregation was absent, health-filtered SQL pagination was absent, and stored coverage ordering disagreed with derived coverage. The API assertion initially also exposed a test setup omission; the test was corrected to heartbeat/register the installation before implementation.
+
+### Review-Fix GREEN Evidence
+
+Focused review and compatibility tests:
+
+```text
+python -m pytest central/tests/test_operational_view_models.py central/tests/test_control_plane.py central/tests/test_jobs.py central/tests/test_blocked_products.py -q
+```
+
+Result: `30 passed`.
+
+Full central suite:
+
+```text
+python -m pytest central/tests -q
+```
+
+Result: `253 passed, 1 warning`.
+
+The warning remains the pre-existing `openpyxl` print-area warning. The test environment also emits the existing `pytest-asyncio` fixture-loop-scope deprecation warning.
+
+Additional verification:
+
+```text
+python -m compileall -q central/app central/scripts
+git diff --check
+```
+
+Both completed without errors; Git's LF/CRLF normalization notices are environmental only.
+
+### Review-Fix Commit
+
+Implementation commit: `e35417f` (`fix: close task 2 operational review findings`).
+
+### Review-Fix Concerns
+
+- Legacy blocker rows intentionally remain tenant-level because their installation ownership is unavailable; they render `-`/null server context and are never assigned a representative installation.
+- The existing non-failing pytest environment warnings remain as noted above.