Skip to content

test: safety net for decomposing the largest files (PR 1) - #1510

Open
abrichr wants to merge 20 commits into
mainfrom
refactor/safety-net
Open

abrichr wants to merge 20 commits into
mainfrom
refactor/safety-net

Conversation

@abrichr

@abrichr abrichr commented Oct 24, 2026 •

Copy link
Copy Markdown
Member

Summary

This is PR 1 of the decomposition plan (packages/flow/docs/design/DECOMPOSITION.md, section 4.3). It adds the safety net that every later move is checked against. It changes no production code.

  • Size ratchet. scripts/check_file_size_ratchet.py runs in Python CI through tests/test_file_size_ratchet.py. It fails when a listed file grows past its budget in the plan's table. A PR that adds net lines raises the budget in its own diff, with a one-line reason. The table and section 6 came from docs(flow): decomposition plan for the largest files #1497. This PR adds the script and the test; there is only one ratchet.
  • Moved-code verifier. scripts/verify_moved_code.py checks that a move commit only relocates code. Every function, method and statement in the base must exist with an identical AST in the head. New code may only be imports, re-exports and protocol stubs. It also lists test patches that a move would make stale.
  • Import surface. packages/flow/tests/test_import_surface.py pins the names that replayer, __main__, qualification and ir provide. It also pins the 276 methods of Replayer.
  • Golden runs. packages/flow/tests/golden_runs/ runs nine scenarios with the real Replayer and PlaywrightBackend against MockMed. Each run's normalized report, file hashes, backend action trace and live app state must match the baseline.
  • Coverage. The non-gating flow-coverage job combines the unit shards' coverage data and reports safety-path coverage in the job summary.

This round

  • Merged with main. The one conflict was in packages.yml. Main's xdist shards, fast lane and single critical e2e run are kept. The golden-runs step and the coverage job are added on top.
  • Linux baselines regenerated. The old baselines dated from October 24, so they were deleted. A temporary commit turned on write mode. A workflow_dispatch Packages run (41704998213) then recompiled the MockMed bundle and wrote all nine baselines on Linux with Playwright 1.61.0. Write mode runs each scenario twice and refuses results that differ; all nine matched. The baselines were copied unchanged from the flow-golden-runs artifact, and the temporary commit was reverted. Two later runs in compare mode reproduced all nine: 41715793206 and 41733145873, the second at the current head. No baseline was edited by hand.
  • Budgets at main's sizes. The table now holds each file's line count on current main. replayer.py is at 19,303 lines. Main keeps changing these files, so re-run python scripts/check_file_size_ratchet.py just before merging and update any number it reports.
  • Import pins. The snapshot was regenerated. It now pins 276 Replayer methods, up from 260. No name was removed.
  • Coverage on nightly and dispatch runs only. Collecting coverage slows the unit suite by about a third. With coverage in every shard, a full-lane shard hit its 40-minute limit at 99%. Pull requests and main's post-merge runs now collect no coverage, so they take no longer than before. Scheduled and dispatched runs collect it, with a 60-minute limit. In run 41733145873 the shards took 45 and 44 minutes.
  • Safety-path coverage is 86.11% (run 41733145873). The job still does not gate.
  • Hashed coverage install. flow-coverage installs coverage 7.15.1 from a new coverage group in .github/ci-tools/uv.lock, with hashes. That is the version packages/flow/uv.lock pins. The earlier unhashed pip install failed tests/test_ci_locked_installs.py.
  • Public artifact inventory. packages/flow/public-artifacts.json now lists the baseline files.

Merge with main, round 2 (head 5daa5b9)

  • Merged main again (13 commits). The only conflict was in packages.yml, in the flow-unit job. Main now picks shard files with scripts/ci/flow_shards.py and runs -n logical. The merge keeps both, plus this PR's golden-runs exclusion from the shard list, the COVERAGE_* env, the upload step and the flow-coverage job. actionlint passes.
  • Budgets raised to main's sizes. Three files grew on main after the last round: replayer.py by 5 lines (to 19,529), __main__.py by 320 (to 13,235) and playwright_backend.py by 83 (to 5,282). This PR leaves all three unchanged from main. The other seven budgets already matched.
  • Import pins regenerated with OPENADAPT_WRITE_IMPORT_SURFACE=1. _refuse_rollback_durable is dropped from the __main__ pins because Rollback on durable runs, and after a crash (#1699) #1772 removed it on main and nothing references it any more. The five names Rollback on durable runs, and after a crash (#1699) #1772 added are now pinned.
  • Golden baselines not regenerated. Nothing in this merge changes the MockMed bundle or replay output on purpose, and the scenarios skip on macOS because the baselines were recorded on Linux. The golden-runs step at this head will show whether they still match. If they don't, they need the write-mode cycle, not hand edits.
  • Desktop failure. At the previous head, src/screens/Stops.test.tsx failed in the Desktop frontend job ("answers the current stop with one key"). This PR doesn't touch packages/desktop. The same timing flake is what desktop: prepare review card input before paint #1825 fixes.

Tested at this head: python scripts/check_file_size_ratchet.py passes; root pytest tests gives 805 passed, 14 skipped; flow tests/test_import_surface.py gives 6 passed and tests/golden_runs gives 5 passed, 9 skipped (the Linux-only scenarios); ruff passes on the golden-runs tests.

Notes for review

  • generate/verify_contract.py (4,157 lines) is now larger than terminal_verification_v2.py (3,663), which is in the table. The table keeps the ten files from docs(flow): decomposition plan for the largest files #1497. Swapping one in is a separate choice.
  • In run 41733145873, src/screens/NeedsYou.test.tsx failed in the Desktop frontend job. This PR does not touch packages/desktop, so the failure is outside it.

Test plan

  • pytest tests/test_file_size_ratchet.py tests/test_verify_moved_code.py (13 passed)
  • Workflow tests: test_ci_locked_installs, test_ci_required_check, test_nightly, test_packages_concurrency, test_newest_green (52 passed)
  • CI wrote the Linux baselines in write mode, each scenario twice with identical results (run 41704998213)
  • CI reproduced them in compare mode (runs 41715793206 and 41733145873)
  • Flow lint, both unit shards, critical and agent frameworks passed (run 41733145873)
  • Python CI passed (run 41733150538)

abrichr and others added 6 commits October 24, 2026 14:48
The safety net for the decomposition plan (DECOMPOSITION.md, PR 1):
- scripts/check_file_size_ratchet.py keeps the ten largest files from
  growing past the budget table in the plan;
- scripts/verify_moved_code.py proves a move commit only relocates code
  (identical ASTs, new code limited to imports, re-exports and protocol
  stubs) and lists test patches a move would make stale;
- tests/test_import_surface.py pins the names replayer, __main__,
  qualification and ir provide, and Replayer's methods.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each flow-unit shard keeps its coverage data; flow-coverage combines them
and writes the safety-path number to the job summary. It is not in
packages-gate: the floor is set at the measured value once known.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Nine scenarios replay one committed MockMed triage bundle under different
drifts through the real Replayer and PlaywrightBackend: clean passes (with
and without the structural rung), identity halts (missing, look-alike,
growth, sort), a modal postcondition halt, the irreversible-step risk gate
and theme healing. Each compares the normalized report, the hash of every
file the run wrote, the backend action trace and the live app state with a
baseline. Writing a baseline runs the scenario twice and refuses values
that differ between identical runs.

Baselines are Linux results generated in the flow-critical job; unit
shards skip the scenarios. CODEOWNERS covers the packages the plan adds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reverted once the baselines from this run are committed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Baselines and the recorded MockMed triage bundle come from the
flow-critical job of CI run 40123224669 (write mode, each scenario run
twice and identical). Ruff skips the bundle's generated workflow.py, which
is part of the bundle digest. Ratchet budgets reset to origin/main
ecc162e.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The baselines are committed; golden runs now compare.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
abrichr and others added 10 commits October 25, 2026 00:11
…larations

A protocol attribute such as Optional[dict[str, Any]] was read as an
assignment because its dump contains 'value='. Statements now record
whether they only declare (an annotation with no value, or a docstring)
when they are parsed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ode reads

A moved method reads module-level names from its new module, so a test
that patches old_module.name (an import such as resolve, or a constant)
no longer reaches it. The verifier now tracks the names each moved
function reads and flags a test that patches one through the old module
without also patching it where the moved code reads it. Found by the
first large replayer move, where a scroll-readiness test patched
runtime.replayer.resolve.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflict in packages.yml: keep main's xdist shards, fast lane and single
critical e2e run; re-add the golden-runs step, the per-shard coverage data
and the non-gating flow-coverage job on top.
Budgets are each file's line count on main at 213f3d9. The snapshot adds
the names and Replayer methods main gained (276 methods); none were removed.
They were written against main of October 24. CI regenerates them, bundle
included, from the merged tree in the next commits.
Reverted once the baselines from this run are committed.
Written by Packages run 41704998213 (workflow_dispatch, flow core critical,
OPENADAPT_GOLDEN_WRITE=1, Playwright 1.61.0). In write mode each scenario
ran twice and the two results were equal before the baseline was written.
Copied unchanged from the flow-golden-runs artifact.
…selines

flow-coverage now installs coverage 7.15.1 (the version packages/flow/uv.lock
pins) from .github/ci-tools/uv.lock with hashes, as
tests/test_ci_locked_installs.py requires. public-artifacts.json lists the
committed golden baselines (regenerated with
--write-public-artifact-inventory).
With --cov in every flow-unit shard, the full lane's shard 2 hit its
40-minute limit at 99% (Packages run 41715793206): coverage slows the suite
by about a third. Pull requests and main's post-merge run now collect none;
a schedule or a dispatch collects it, with a 60-minute limit, and
flow-coverage reports it.
@abrichr
abrichr marked this pull request as ready for review November 4, 2026 00:46
Conflict in packages/flow/pyproject.toml: take main's comment about the
benchmark harness and keep tests/golden_runs/baselines in extend-exclude.
replayer.py grew to 19524, __main__.py to 12915, ir.py to 6352,
recorder.py to 4823 and playwright_backend.py to 5199 on main while this
waited. The snapshot adds the 4 names main added; none were removed.
… merge

Three files grew on main since the budgets were set (replayer.py +5,
__main__.py +320, playwright_backend.py +83); section 6 sets each budget to
main's size when this PR merges. The import-surface snapshot drops
_refuse_rollback_durable, which #1772 removed on main with no remaining
references, and adds the names #1772 added.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant