194 lines
8.9 KiB
Markdown
194 lines
8.9 KiB
Markdown
# Task 2 Report: Bounded Operational Query View-Models
|
|
|
|
## Status
|
|
|
|
Implemented Task 2 only. The control-plane query layer now supplies bounded operational view-models while preserving the existing JSON keys and technical health derivation. No fiscal logic, database schema, Firebird access, or remote write capability was changed.
|
|
|
|
## RED Evidence
|
|
|
|
Added `central/tests/test_operational_view_models.py` before changing production code.
|
|
|
|
Command:
|
|
|
|
```text
|
|
python -m pytest central/tests/test_operational_view_models.py -q
|
|
```
|
|
|
|
Result before implementation: `6 failed`.
|
|
|
|
The failures were the expected missing-contract failures: `dashboard_summary` did not accept `now`, `list_installations` did not accept `sort`, `list_jobs` did not accept `client`, `list_blockers` did not accept `client`, and `blocker_detail` did not exist. This confirmed the tests exercised the new behavior rather than existing behavior.
|
|
|
|
## GREEN Evidence
|
|
|
|
Focused Task 2 tests:
|
|
|
|
```text
|
|
python -m pytest central/tests/test_operational_view_models.py -q
|
|
```
|
|
|
|
Result: `6 passed`.
|
|
|
|
Focused compatibility tests:
|
|
|
|
```text
|
|
python -m pytest central/tests/test_operational_view_models.py central/tests/test_control_plane.py central/tests/test_jobs.py -q
|
|
```
|
|
|
|
Result: `21 passed`.
|
|
|
|
Central regression suite:
|
|
|
|
```text
|
|
python -m pytest central/tests -q
|
|
```
|
|
|
|
Result: `248 passed, 1 warning`.
|
|
|
|
The warning is the pre-existing `openpyxl` print-area warning from `central/tests/test_bootstrap_v1_extra.py`. The pytest environment also emits the existing `pytest-asyncio` fixture-loop-scope deprecation warning.
|
|
|
|
Additional verification:
|
|
|
|
```text
|
|
git diff --check
|
|
```
|
|
|
|
Result: no whitespace errors. Git reported only its existing LF/CRLF normalization notice for the edited Python files.
|
|
|
|
## Changed Files
|
|
|
|
- `central/app/control_plane.py`: added bounded dashboard aggregates and attention items; enriched installation, job, and blocker view-models; added filters, stable pagination, worst-coverage sorting, presentation fields, and bounded blocker detail relations.
|
|
- `central/tests/test_operational_view_models.py`: added focused TDD coverage for health reconciliation, distinct safe-blocked products, attention ordering/cap, fiscal/technical separation, job identity/duration, blocker filters/priorities, and detail relations.
|
|
|
|
## Contract Notes
|
|
|
|
- Dashboard health counts preserve lowercase compatibility keys and add uppercase `health_counts`.
|
|
- `safe_blocked_products` counts distinct active `(tenant_id, product_code)` pairs; `open_blockers` counts active rows.
|
|
- Safe fiscal blockers do not alter `technical_health`.
|
|
- API timestamps remain ISO UTC values; presentation fields use Task 1 helpers.
|
|
- Collection queries use joins/aggregates. Blocker detail related events, jobs, and audit rows are bounded and bulk-loaded.
|
|
- Existing `resolved` behavior remains supported while `status` adds explicit `OPEN`, `RESOLVED`, and `ALL` handling.
|
|
|
|
## Commit
|
|
|
|
Implementation commit: `d729710` (`feat: add bounded operational view models`).
|
|
|
|
## Concerns
|
|
|
|
- `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 Re-Review Fix Report
|
|
|
|
### Findings Fixed
|
|
|
|
- Unified fiscal completion rounding with deterministic one-decimal `ROUND_HALF_UP` behavior in Python and SQL. The SQL ratio is explicitly numeric before two-argument `ROUND`, preserving SQLite/PostgreSQL compatibility.
|
|
- Extended `blocker_fingerprint` with an optional installation identifier. Legacy calls without an installation retain the original hash; authenticated upserts scope both generated and supplied fingerprints to the exact installation, so identical reports from two installations remain distinct.
|
|
- Normalized blocker attention candidate fields with SQL `UPPER`, matching the normalized breakdown behavior for lowercase `cfop`, `st`, and other required fields.
|
|
|
|
### Re-Review RED Evidence
|
|
|
|
Command after adding the three regression tests:
|
|
|
|
```text
|
|
python -m pytest central/tests/test_operational_view_models.py::test_worst_coverage_sort_cannot_disagree_with_one_decimal_display central/tests/test_operational_view_models.py::test_lowercase_required_blocker_field_reaches_attention_candidates central/tests/test_blocked_products.py::test_authenticated_installations_get_distinct_fingerprints_for_identical_blockers -q
|
|
```
|
|
|
|
Result before implementation: `3 failed`.
|
|
|
|
The failures reproduced the SQL/Python 6.3 versus 6.2 ordering mismatch, omitted lowercase blocker attention, and deduplication of identical blocker reports across installations.
|
|
|
|
### Re-Review GREEN Evidence
|
|
|
|
Focused Task 2/control/blocker 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 central/tests/test_operational_state.py -q
|
|
```
|
|
|
|
Result: `37 passed`.
|
|
|
|
Full central suite:
|
|
|
|
```text
|
|
python -m pytest central/tests -q
|
|
```
|
|
|
|
Result: `256 passed, 1 warning`.
|
|
|
|
The warning remains the pre-existing `openpyxl` print-area warning; the environment also emits the existing `pytest-asyncio` fixture-loop-scope deprecation warning.
|
|
|
|
PostgreSQL SQL compatibility check:
|
|
|
|
```text
|
|
python -c "from sqlalchemy import select; from sqlalchemy.dialects.postgresql import dialect; from central.app.control_plane import _fiscal_completion_expression; print(select(_fiscal_completion_expression()).compile(dialect=dialect()))"
|
|
```
|
|
|
|
Result: the expression compiles as numeric SQL using two-argument `ROUND`.
|
|
|
|
### Re-Review Fix Commit
|
|
|
|
Implementation commit: `2ee271c` (`fix: close remaining task 2 review findings`).
|
|
|
|
### Re-Review Concerns
|
|
|
|
- Legacy no-installation payloads intentionally retain their original fingerprint and tenant-level behavior; authenticated installation-scoped payloads use the new namespace.
|
|
- 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.
|