Repository navigation
Conversation
…ven its request timeout (failing) The live parity check's US household puts income_tax on the person, which /calculate refuses, and the check gives /calculate-full 90 s although the candidate lets a request run for 300 s. Both failed the release's staging checks. These tests send each parity household through the Public API's own input checks with the installed country models, and require the check's read timeout to be longer than CLOUD_RUN_TIMEOUT. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…calculate-full The release's live staging checks failed twice: - us-single-california: /calculate answered 400, "Household variable `income_tax` belongs on `tax_units`, not `people`". policyengine_us defines income_tax on the tax unit, and /calculate has refused a variable on the wrong entity since May; the simulation service's household function refuses it the same way. The household now asks for it on its tax unit. - us-single-reform-added-variables: httpx.ReadTimeout after 90 s on /us/calculate-full. The candidate's request log shows that request answered 200 after 223.8 s; production's /calculate-full takes from 50 s to Cloud Run's 300 s request timeout. The check now waits 320 s for the Public API's own calculation, so it gets the candidate's answer, a 504 included. The check also reads the v2 job's status before calling the Public API, so a failed job is reported without waiting minutes for /calculate-full. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…routes The unit test called prepare_household_calculation itself, so it skipped the situation parsing /calculate and /calculate-full run before calculating, and any check added to the routes later. It now registers the household routes on a test Flask app with the installed country models and replaces only the calculation, which takes minutes for /calculate-full. On the old us-single-california household it fails with the body staging answered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The routes' answer under test is the prepared household, with deprecated inputs dropped and missing variables added, and a refusal during the calculation itself, such as an SPM error, is outside the test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4288 +/- ##
==========================================
+ Coverage 92.69% 92.74% +0.05%
==========================================
Files 329 329
Lines 18783 18783
Branches 3114 3114
==========================================
+ Hits 17410 17420 +10
+ Misses 818 809 -9
+ Partials 555 554 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4287
Summary
The live staging household checks added in #4274 failed in the release run on
cd513dacc(push.yml run 43075278725), so no release of the Public API reaches production until they pass. Two parity cases failed, for two different reasons:us-single-california:/us/calculateanswered 400, "Household variableincome_taxbelongs ontax_units, notpeople".policyengine_usdefinesincome_taxon the tax unit, and/calculatehas refused a variable on the wrong entity since3a59b4d14. The simulation service's household function has the same check. The household now asks forincome_taxon its tax unit.us-single-reform-added-variables:httpx.ReadTimeouton/us/calculate-fullafter 90 s, the test client's read timeout. The candidate's request log shows that request answered 200 after 223.8 s. Production's/calculate-fullrequests from the past week took between 50 s and 296 s, and some got Cloud Run's 504 at its 300 s request timeout. The parity check now waits 320 s (V1_CALCULATION_TIMEOUT_SECONDS) for/calculateand/calculate-full. That is longer thanCLOUD_RUN_TIMEOUT, so the check gets the candidate's own answer.The parity check also reads the v2 job's status before calling
/calculate. A failed job is then reported without waiting minutes for the Public API's own calculation.Known limit: the added-variables case now passes only if staging's
/calculate-fullanswers within Cloud Run's 300 s request timeout. Its one staging measurement is 224 s; production has answered in up to 296 s, and has sometimes given a 504. A slower staging run would block a release on the Public API's own latency rather than on a parity difference.Tests
tests/unit/v2/test_live_household_parity_requests.py, committed failing first:test_the_public_api_accepts_each_parity_householdsends each of the six parity households, ascalculate_with_v1sends it, to the real/calculateand/calculate-fullroutes, registered on a test Flask app with the installed US and UK models. The routes check the inputs and parse the situation as in the Public API; only the calculation is replaced, by an answer that returns the household as the routes prepared it. It failed onus-single-californiawith the staging message.test_the_check_waits_longer_than_the_candidate_lets_a_request_runrequires the read timeout of both requests to exceedCLOUD_RUN_TIMEOUT, read from.github/scripts/cloud_run_env.sh. It failed with the client's own timeout.alembic -c alembic-v2.ini checkon a disposable PostgreSQL 17: no new operations, one head0c136a278f29.lint-imports, the rules registry, the quality guards, ruff, ruff format, shellcheck,uv lock --checkand mypy pass.Environment variables and secrets
The change reads no new variable.
run_live_staging_tests.shchanged only in a comment. The live step receivesAPI_BASE_URL,CI_SERVER_ACCESS_TOKENandSTAGING_API_TEST_PROBE_IDthrough itsenv:; the failed run's log shows all three set. The new unit test readsCLOUD_RUN_TIMEOUTfromcloud_run_env.sh's default, which no job inpush.ymloverrides.🤖 Generated with Claude Code