fix: unify run_python persistence semantics — writes always persist - #247
Merged
Conversation
…e save flag Local and Remote run_python had divergent 'save' semantics: Local persisted writes unconditionally (save only gated the build), Remote silently discarded writes without save=True. The v0.5.1 persona unification exposed this as agent instability in the cloud E2E (vibe agent 'forgetting' save=True). New unified contract: - File writes always persist — no 'unsaved' state, no flag to forget - PPTX artifact rebuilds automatically when the deck changed (cheap build) - measure_slides is the only trigger for the expensive verification pass - save is accepted but ignored (deprecation note in result) - Remote write-back is diff-based (baseline snapshot), fixing the stale-copy clobber risk; read-only users run without persistence (readOnly note) - run_style_python (Remote) unified the same way: style.html persists automatically when changed personas/guides/wiring/docs updated (repo-wide save= mentions: 0). tests/test_run_python_semantics.py pins the semantics on both adapters. SPEC: 20260801-1122_run-python-unified-semantics Progress: Phase 0-2 complete; make all 628 passed / lint clean Next: redeploy cloud stack, user re-runs compose E2E (v0.5.1 gate)
…eparation, files collision guard Review findings (all 6 addressed): - High: expensive SVG render/compose ran on deck_changed without measure_slides (contract violation), and compose/preview skipped when measuring an unchanged deck. Introduced _post_processing_plan (pure decision table: build/artifact/verify) and rewired the gates; no more measure-error noise without a measure request - High: files=[...] staged by basename could shadow deck.json and persist via the unconditional write-back — collision names now rejected - Medium: artifact refresh failures now surface as result.pptx_error with best-effort compensation delete of the orphaned upload - Medium: run_style_python warns when style.html was written without style_name (no more silent discard) - Low: import-pptx.md stale generate_pptx freshness claim updated - Low: _upload_deck_workspace return annotation fixed Added real-wiring branch tests for the 4-pattern contract matrix (monkeypatched rig around the actual run_python). SPEC: 20260801-1122_run-python-unified-semantics Progress: review fixes complete; make all 640 passed / lint clean Next: local compose smoke → cloud redeploy → user re-E2E
…nged slide files Unconditional rewrites on diagnostics bumped mtimes every call, falsely marking the deck as changed and rebuilding output.pptx on read-only runs (caught by the local compose one-pass check). Rewrite only when sanitization actually changed the content. SPEC: 20260801-1122_run-python-unified-semantics Progress: local compose one-pass verified (build+preview OK, no read-only rebuild) Next: cloud redeploy → user re-E2E
…on tests, scoped files guard - Add regression tests for the lint rewrite guard (diagnostics-only → no rewrite/no rebuild; _spAutoFit sanitize → rewrite + single rebuild) - Apply the same cleaned != slide_data guard to Remote lint (no reserialization for diagnostics-only results; byte-identical test) - Scope the staged-files collision guard to deck sessions; exact match for file entries (deck.jsonl no longer rejected), prefix match for directories; extracted _validate_staged_files for unit testing - Fix import-pptx.md 'download link' claim and remote generate_pptx docstring to match the actual return and finalize/handoff role SPEC: 20260801-1122_run-python-unified-semantics Progress: re-review conditions met; make all 644 passed / lint clean Next: cloud redeploy with this commit → user re-E2E
… on refresh The automatic artifact refresh uploads a new UUID key per build-relevant change; one E2E session accumulated 19 orphaned PPTX objects (only the newest is referenced by the deck record). update_deck now returns the previous attribute values (ReturnValues=UPDATED_OLD) and both refresh paths (run_python auto-refresh, generate_pptx) delete the superseded artifact after a successful record update — best effort, with the existing lifecycle rules (pptx/ 90d, noncurrent 30d) as backstop. SPEC: 20260801-1122_run-python-unified-semantics Progress: E2E passed (deck 5fa63146); artifact cleanup fixed; 646 passed Next: PR → merge → v0.5.1 tag
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
1 participant
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.
Summary
run_pythonhad divergentsavesemantics between Local (build trigger; writes always hit disk) and Remote (persistence gate; writes silently discarded without it). After the v0.5.1 persona unification, local-oriented wording became the cloud instruction and composer/vibe agents intermittently "forgot"save=True, losing work (deck ba8d79da).This PR removes the failure class instead of prompting around it:
saveis deprecated — accepted and ignored with a deprecation note (no caller breakage)output.pptx/pptxS3Keyauto-refresh on build-relevant changes (deck.json, presentation.json, specs/outline.md, slides/, includes/) — cheap build, no rendermeasure_slidesis the only trigger for expensive verification (SVG render + measure + preview + compose)generate_pptx= explicit finalize/handoff (WebP previews + KB sync + full-deck warnings report)update_deckreturnsUPDATED_OLD) — auto-refresh doesn't accumulate orphansfiles=[...]names colliding with the deck workspace are rejected (deck sessions only, exact/prefix split); artifact refresh failures surface asresult.pptx_errorwith compensation delete;run_style_pythonwarns when style.html is written withoutstyle_name; lint pass no longer rewrites unchanged slide files (false rebuild fix)Contract
Implemented as a pure decision table (
_post_processing_plan) + real-wiring branch tests.Tested
make all: 646 passed / lint clean; 30 new tests intests/test_run_python_semantics.py(contract matrix via monkeypatched rig around the actualrun_python, files collision, artifact failure surfacing, superseded-artifact cleanup, lint rewrite guard, Local/Remote save-flag compat)save,output.pptxauto-followed changes,pptxS3Keypoints at the newest artifactDocs
Personas (vibe/spec/single/composer), import-pptx guide, style_remote wiring, architecture.md updated — no
save=mentions remain. CHANGELOG under [Unreleased].