[PavelMakarchuk]

Fixes #4043

Related: #3946 (the 500 on the same PATCH, not explained by this change).

What and why

GET /{country_id}/report/{id} serves a report stored under an outdated report-output cache version as the current report for the same simulations and year, created if needed, with the requested ID as response_id. PATCH /{country_id}/report selected the record by ID, so a client saving the result it was served wrote it to the outdated record, and the served report stayed pending for ever.

ReportOutputService.update_report_output now resolves an outdated ID the way resolve_report_output does: it updates the current report for the same simulations and year (creating it if none was served yet), and the view keeps the requested ID as response_id, so the response's id is the one the client sent. The outdated record is left as it is. Current IDs behave as before.

This matters now because #4024 changed the cache version of every older v1 report (UK and US), so each one goes through this path the next time it opens.

Not covered

The 500 in #3946 (reports 4501 and 4729) does not reproduce on the SQLite test database, before or after this change. Its cause still needs the production log line "Unexpected error updating report #… for country …".

Overlap: #4029 (draft) also changes PATCH /{country_id}/report to keep complete results final. The two changes touch different parts of update_report_output; whichever merges second needs a small rebase.

Tests

• test_update_with_outdated_id_completes_the_served_current_report and test_update_with_outdated_id_creates_the_current_report_when_none_was_served (service). Both fail on master.
• test_report_patch_with_outdated_id_completes_the_report_get_serves (route): GET, PATCH and GET again with the outdated ID.
• pytest tests/unit/test_simulation_and_report_routes.py tests/unit/services: 631 passed. ruff check, ruff format --check and scripts/run_quality_guards.py pass (shellcheck not installed locally).

No API contract change: the request and response shapes are unchanged.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.82%. Comparing base (ed036b4) to head (9ac4902).
⚠️ Report is 31 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #4044       +/-   ##
===========================================
+ Coverage   50.96%   87.82%   +36.86%     
===========================================
  Files         242      258       +16     
  Lines       14527    15954     +1427     
  Branches     2334     2622      +288     
===========================================
+ Hits         7403    14011     +6608     
+ Misses       6501     1278     -5223     
- Partials      623      665       +42     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[PavelMakarchuk]

Tested on MySQL as well as SQLite. I ran a local MySQL with the schema from alembic -c alembic-v1.ini upgrade head, then the report service and route tests (test_report_output_service.py, test_simulation_and_report_routes.py, test_report_run_service.py): 42 passed.

I also replayed the production case with report 4729's real UK result (/uk/economy/98850/over/1?region=uk&time_period=2027), sent as the app sends it (output as a JSON string), against an outdated record that already had a complete run:

Code	PATCH	GET afterwards
master	200	pending
this PR	200	complete, with the output

The 500 from #3946 did not reproduce on MySQL either, so its cause is in production state or setup. It still needs the production log line "Unexpected error updating report #4729 for country uk".

🤖 Generated with Claude Code

[anth-volk]

Approving: a PATCH that names an outdated report ID now resolves it the same way GET does, so the result reaches the report GET serves, and current IDs behave as before. Before merging, please make these four changes:

1. Validate the requested ID, or take it from the record. The PATCH route passes payload.get("id") to update_report_output unchanged, and the new code copies it into response_id, which ReportOutputView types as int | None. A PATCH with {"id": "1", …} for an outdated report returns "id": "1" as a string, while a current report returns the stored integer. Either check it as GET does (the integer check in _get_report_output), or set response_id from the outdated record's own id.
2. Update the documentation. The update_report_output docstring lists ValueError only for the running-status and SPM-provenance cases. The new path can also raise it from _require_simulation, when an outdated record points to a simulation that no longer exists, and the route returns that as a 400. The PATCH /{country_id}/report route docstring and its OpenAPI description still say the request updates the record with that ID, and the GET description doesn't say it can serve a different record.
3. Cover the other two cases the new path takes. The new tests use UK reports with status="complete" only. Please add a test for status="error", which the app sends when a report fails, and one for a US economy report, where _update_report_output checks SPM provenance on the current record (#3946 is a US report).
4. Share the outdated-record setup. The steps that build an outdated record (set the report_output schema version to 2, compute the cache version, set it back to 3, insert a ReportOutput) now appear three times: in the existing US test in test_report_output_service.py, in the new create_outdated_uk_report helper, and in the new route test, which also imports constants inside the test function. One shared helper or fixture would keep them consistent.

[PavelMakarchuk]

Thanks @anth-volk. All four are in 9ac49029:

1. Record ID. response_id now comes from the outdated record's own id, so it is always the stored integer. A new route test sends {"id": "<id>"} as a string and checks that the response id is the integer.
2. Documentation. The update_report_output docstring lists the new ValueError (an outdated record whose simulation no longer exists). The PATCH and GET route docstrings and their OpenAPI descriptions now say that an outdated ID updates or serves the current report, under the requested ID.
3. Other cases. New tests: status="error" on an outdated UK ID (the current report records the error; the outdated record is unchanged), and a US economy report on an outdated ID, once with valid compact SPM provenance (completes) and once without (raises, and the current report stays pending).
4. Shared setup. tests/fixtures/services/report_output_fixtures.py::insert_outdated_report_output builds the outdated record. The existing US test, the service tests and the route tests all use it, and the in-function constants import is gone.

Locally: ruff check and ruff format --check pass, and pytest tests/unit passes (3,303 tests).

[anth-volk]

Good to merge once CI passes