docs: append task 2 re-review report
This commit is contained in:
@@ -77,6 +77,63 @@ 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.
|
- `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.
|
- 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
|
## Task 2 Review Fix Report
|
||||||
|
|
||||||
### Findings Fixed
|
### Findings Fixed
|
||||||
|
|||||||
Reference in New Issue
Block a user