Skip to content

Skip duplicate JSON conversion for prepared checkpoints after #11 - #12

Merged
github-actions[bot] merged 2 commits into
mainfrom
codex/checkpoint-prepared-main-20261016
Oct 16, 2026
Merged

github-actions[bot] merged 2 commits into
mainfrom
codex/checkpoint-prepared-main-20261016

Conversation

@D-sudoasd

@D-sudoasd D-sudoasd commented Oct 16, 2026 •

Copy link
Copy Markdown
Owner

Final status — 2026-10-16

Merged into main as 96c253e65adb6b58ecab9127dc6784d0d7e8409f. The complete main Git tree matches the verified head 5f76bc89b89a2586ea9d14801b94be4821a16802.

The Windows/Python 3.12 hosted-runner dependency collision was fixed in 5f76bc8 by installing and running the matrix in an isolated .venv-project, also used by the existing Windows desktop launcher. Dependency pins, pip check, scientific code and recovery identity checks were not relaxed.

CI run 38855207424 passed all eight jobs: lint, all six Linux/Windows Python 3.11–3.13 matrix jobs, and wheel smoke. Actual pytest, dependency checks and CLI smoke passed in every matrix job; all three Windows desktop-launcher checks passed.

At the user's explicit request, the merged #11/#12 branches, the superseded #10 branch and the temporary maintenance branch were deleted with exact-SHA leases after checking the merged file tree and PR states. Remote branches now contain only main, with no open PRs. #10 remains closed and unmerged; its original commit and discussion remain available. The temporary maintenance workflow was not merged into main. No existing Agent working directory was modified.

Implementation

Before this change, checkpoint snapshots on #11 main still traversed scientific history a second time in the writer. This PR preserves #11's scalar-copy specialization and benchmark, and retains only #10's independent prepared-payload serialization change. Public mappings retain generic JSON conversion; internal frame/run metadata is normalized at construction, including retained partial-resume records and overflowed array summaries.

Based on a911c2e9946652fe742e4dd054ebf61080ff3ee5; supersedes #10. Exact LF/CRLF predecessor source pairs from #11/#10 are added without relaxing other producer, dependency, analyzer, input/config or native-array gates. Full histories, float precision, commit frequency and synchronous durability remain unchanged.

Earlier focused validation

170 focused tests passed, 3 Win32-only metadata checks skipped on Linux; default Ruff rules and diff checks passed. Tests include every accepted source pair in full/partial native recovery, NPZ/provenance preservation, mutable history, prepared/public equivalence, failure cleanup and real process exits after fsync/replace. Independent fixed-clock main/candidate subprocesses matched all four checkpoint generations by nested field/type/float.hex except verified actual producer identity.

Synthetic performance observations

Single paired 24-frame synthetic outer-timed replay: 15.10987 -> 8.96429 seconds, 25 synchronous commits each, strict readback passed. Stage-timed partial replay: 6.25683 -> 6.59625 seconds despite generic conversion decreasing 1.70838 -> 0.00783 seconds; no general partial-resume speedup is claimed. No real 121-frame production measurements or scientific fitting were performed. These observations are not combined with previous PR percentages, and no performance or production-fitting replay was repeated during final closeout.

Details, comparison decisions and reproduction: docs/validation/checkpoint_prepared_payload.md. The original implementation used an isolated clone; final CI repair and closeout used GitHub operations and an isolated Actions runner. The connected workspace API was unavailable and no existing work environment was changed.

@D-sudoasd D-sudoasd left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review against main a911c2e. I found this replacement already in progress while comparing #10/#11, so I did not create another implementation branch, alter an existing workspace, or repeat the reported focused validation.

The retained scope is appropriate: #11's exact-built-in list/dict specialization and benchmark remain intact; #10's independent contribution is the prepared-payload writer. Frame/run metadata normalization, retained partial-resume metadata, and overflowed array-summary min/max are necessary companions to bypassing the final generic conversion. The synchronous write/flush/fsync/close/replace sequence, public-mapping conversion, full-history rebuild, and forward-only exact source/builder gates remain present. The expanded storage tests exercise every accepted predecessor pair and both full/partial native restoration, and abrupt-exit tests now cover both public and prepared writes. I have reviewed this code and the published validation record; I have not independently rerun the 170-test command.

CI blocker found: Windows / Python 3.12 failed before pytest. Run 38854235242, job 116822062616, failed at Check dependencies (python -m pip check), not at the checkpoint tests. The job log explicitly reports:

pipx 1.17.12 has requirement filelock>=4.0.9, but you have filelock 3.32.4.

The install log shows runner-preinstalled filelock 4.0.12 being replaced by the repository's existing constraints/validation-py311-313.txt pin, filelock==3.32.4. This is a collision with the hosted runner's preinstalled pipx in the shared interpreter. The Test and smoke steps in that matrix job were skipped, so it cannot be counted as cross-platform validation.

Please keep follow-up on this integration path and isolate the test installation in a fresh virtual environment without system-site-packages, with installation, pip check, pytest, and CLI smoke consistently using that environment. Preserve the dependency pins and the dependency check; do not relax scientific/recovery identities to make CI pass. Do not merge until the affected matrix validation and the normal remaining checks complete successfully. I have not launched a competing CI repair or rerun.

Performance interpretation remains limited to the recorded synthetic cases: the single continuous outer-timed pair improved, but the partial-resume total did not. The generic-conversion reduction alone does not establish a partial-resume speedup, and none of these numbers establish a 121-frame production speedup. Closing #10 with its branch preserved and directing further work to #12 is the correct coordination outcome.

Copy link
Copy Markdown
Owner Author

Taking the user-requested final closeout on this existing PR: fix the hosted-runner dependency collision, complete normal cross-platform CI, then merge only the verified head and remove superseded/merged branches after checking their tips. Current head is still 1460a45; no newer code/discussion was found. The connected workspace API remains unavailable, so no remote working directory is being changed. The CI fix will use an isolated .venv-project so installation, pip check, pytest, CLI smoke and the existing Windows desktop launcher resolve the same environment. Dependency pins and scientific/recovery code will remain unchanged for this fix.

@github-actions
github-actions Bot merged commit 96c253e into main Oct 16, 2026
8 checks passed
@github-actions
github-actions Bot deleted the codex/checkpoint-prepared-main-20261016 branch October 16, 2026 05:43
@github-actions

Copy link
Copy Markdown
Contributor

Final closeout completed at user request. CI isolation fix 5f76bc8 passed the full six-job Linux/Windows Python 3.11-3.13 matrix, including actual pytest, dependency checks, CLI smoke and all three Windows launcher checks; lint and wheel smoke also passed. CI run: https://github.com/D-sudoasd/WingSAXS/actions/runs/38855207424 . PR merged as 96c253e; main's complete Git tree was checked against the verified head and is identical. The merged #11/#12 branches, superseded #10 branch and this temporary maintenance branch were removed together with exact-SHA leases. No temporary workflow was merged into main and no production fitting or performance rerun was performed. Existing synthetic-performance limits still apply.

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