Repository navigation
Conversation
…laim identity reconstruct_workflow_status collapsed the step ledger's terminal rows by step_key alone, so one map child's failed retry was inherited by its still-live pending siblings: derive_workflow_status returned 'failed' for a run whose own authoritative root writer (WORKFLOW_ROOT_MAINTAIN_SQL) kept it running — the intermittent march e2e red. The 'latest terminal' overwrite also relied on physical arrival order (no ORDER BY), so the verdict flipped between samples after a tuple move + ANALYZE. The terminal read now carries the full claim identity (job_id, step_key, map_index, attempt) in a deterministic (attempt, created_at, id) order — the memoized replay's own latest-attempt convention — and the pending-row remap fires only for the row's OWN current claim's terminal: a historical attempt's terminal cannot relabel the node whose claim has moved past it, and a sibling's terminal can never answer for it. Red-first controls (tests/test_wf_status_pins.py), both red on the unmodified base for the defect's exact reason and green after: - test_reconstruction_map_sibling_never_inherits_a_siblings_terminal - test_reconstruction_latest_terminal_is_order_deterministic
…obs rows are the current-node record (the ruling's rework) The ruling (RULING_STATUS_IDENTITY.md) rejects the (job_id, attempt) fix and every exact-tuple latest-terminal projection: a jobs row that is currently pending/scheduled/running is LIVE current state, and no wf_step_ledger terminal may relabel it. The node terminal and its own ledger terminal commit in one transaction (_run_tx1), so a live row beside ledger history is a deliberate live shape — a ladder re-pend, a manual retry, a held/awaited claim, an inner substep/iteration claim — attempt history, never a recoverable partial commit. _status.py: _LEDGER_TERMINALS_SQL and the flow-wide ledger fetch are DELETED (with their ORDER BY — there is no terminal-selection rule to order); ledger_by_job/ledger_attempt_by_job, the ordered overwrite and the pending-node remap are gone. NodeView is built directly from every WORKFLOW_NODES_SQL row; derive_workflow_status, the terminal-crash fold, the absorption handling, _NEVER_GRANTED_SQL and its append are unchanged. Reconstruction is TWO statements and reads no ledger table. Controls: the two aggregate-masked controls are replaced by the ruling's families — failed and succeeded map siblings (exact pending, join phase separate, NULL-vs-0 arbiter check), held step and held loop with inner substep/iteration terminals in both directions, the killed/reclaimed child isolated then joined (both directions), the REAL ladder repend + scheduled→pending sweep, the REAL manual retry/reopen (unchanged attempt, run_epoch advanced), and the terminal node rows' win over missing/stale/contradictory ledger history — each asserting the exact seeded tuples, then the exact reconstruction verdict, then the real WORKFLOW_ROOT_MAINTAIN_SQL root. The query-count pin asserts exactly two statements at both populations and NEITHER may read wf_step_ledger; the heap-motion + ANALYZE control proves byte-identical verdicts with the ordering concept gone.
…, the stale ledger-status docs, the own-claim contradictory-ledger control
1. ruff format on the pins module (the manual-retry assertion's wrap);
ruff check . and ruff format --check . both clean (1885 files).
2. The stale two-source/ledger-status commentary corrected everywhere
the ruling's same-edit requirement reached: _sql_status.py's module
docstring ('the ledger IS the status store' — the rejected model),
the G7 fixture's docstring (the assertion reads the committed node
rows; no ledger), engine.py's skip fan-in comment (the ledger stays
claim/attempt history; the status read is the node rows), the pins
module's section header + the two-sources test's docstring (one
source: the committed node rows; no ledger overlay), and the upgrade
guide's current-behavior line (the reconstruction consults no
ledger; a live row beside ledger history is a deliberate live
shape). The tx1→tx2 crash-window prose and the unrelated two-source
rules (env queues, deploy capacity) are untouched — they name other
mechanisms.
3. The terminal-facts control now seeds every contradictory ledger row
on the NODE'S OWN job id (the arbiter tuple the deleted overlay
keyed), _assert_ledger asserts the owning job id explicitly, both
directions are covered (failed node over its own succeeded ledger;
succeeded node over its own failed AND crashed ledger), the
overlay's absence is asserted in-family (a counting connection may
issue no wf_step_ledger statement), and the real maintenance runs in
the family body (the G7 fixture runs it again at teardown). The
unrelated-job variant is kept, explicitly documented as unrelated
history. Fixture coherence: the failed-map join phase is a real
fan-in (deps_pending=2 + the two wf_edge rows); the held shapes
carry the hold's real faces (the scheduled_at deadline + the
status='held' wf_signals row). RED under the injected own-job
overlay mutant (the overlay-absence assert fires, recording the
shim's ledger SELECT); green on the real tree.
The rework2 review's five residual sites: the agent-fleet user guide's current-behavior line (the step ledger's attempted terminals plus the node rows → re-derived from the committed node rows alone, no ledger read, the one-transaction terminal invariant), the two_sources test's three body comments (the ledger carries it / the two-source rule's second source / the ledger's truth → the node row's own terminal fact and the never-granted read), and the fixture section header (the reconstruction's ledger terminal identity → the one-source law: the ledger is history, not state). The full docs/+tests/ sweep finds no remaining claim that status reconstruction reads the ledger, overlays terminals, or combines two sources for status; the other two-source/ crash-window mentions name unrelated mechanisms (queue-strict evidence, emit backpressure, the tx1→tx2 propagation window, the attempt-chain wait analytics) and stand.
This branch has not been 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 the intermittent
derived failed, not completemarch failure onmainby correctingreconstruct_workflow_status.Defect
reconstruct_workflow_statusfoldedwf_step_ledgerterminal rows bystep_keyonly, with noORDER BY, and projected them ontopendingnodes. Two consequences, both reproduced on real PostgreSQL 18.6:sources.itemmap child was inherited by its still-livependingsiblings, so a live run reconstructedfailedwhile the product's own root writer kept__flow__running.ANALYZEcould flip a verdict between prematurecompleteandfailedfor identical logical rows.Ruling and fix
An architecture ruling established the invariant: a ledger terminal never applies to a currently live node. The current committed
jobsrows are the current-node record;wf_step_ledgeris claim/attempt history and replay data. A node terminal and its own ledger terminal commit in one_run_tx1transaction, so a live row beside ledger history is a deliberate live shape — a retry, a hold, an inner claim — never a partial commit.So the fix deletes the overlay rather than re-keying it:
_LEDGER_TERMINALS_SQL, both ledger maps, the ordered-overwrite loop and thepending-node projection are gone. Reconstruction buildsNodeViewdirectly fromWORKFLOW_NODES_SQLand is now two statements with zerowf_step_ledgerreads.An earlier attempt keyed the ledger by
job_id+ attempt; review demonstrated it collapsed a node's own claim with itsctx.substepand loop-iteration claims, reconstructing held or killed live nodes ascomplete/failed. That approach is discarded, not refined.Controls
Eight control families plus a query inventory, each asserting the exact seeded tuples, then the verdict, then the real
WORKFLOW_ROOT_MAINTAIN_SQLroot: failed and succeeded map siblings (with a NULL-vs-0map-index check), held step and held loop with inner substep/iteration terminals in both directions, killed/reclaimed child isolated and joined, real ladderrepend_attempt+ scheduled→pending sweep, real manual retry/reopen withrun_epoch, terminal node rows winning over their own contradictory ledger history in both directions, the two-statement/no-ledger pin at two populations, and heap-motion +ANALYZEbyte-identity.Non-vacuity, independently reproduced by the reviewer:
4441dd642: map-failed, map-succeeded, ladder-repend, manual-retry and heap-motion red;job_idfix: six isolated reds;ORDER BYmutant: the same six;assert 'complete' == 'failed').Verification
git statusproven unchanged across the run)measured=305.00s bound=866.00sEXPLAIN (ANALYZE, BUFFERS)at 25,601 jobs / 1.2M ledger rows:workflow_nodes19.3 ms overjobs/wf_edge,_NEVER_GRANTED_SQL0.023 ms overjobs— no ledger relation, no sequential scan, no sort growthReviews
2cafeaa8…(REVISE: format, stale prose, vacuous terminal-facts control),90e2e34f…(confirmations plus five remaining prose sites),901c75b4…(CLEAN prose: 0 non-prose lines, AST-identical test module),b638db0b…(attribution: the nine evidence-door reds were a HEAD-mutation race in our own lane, not code)6208b82e…(CLEAN, merge gated on the heavy lanes, which now pass)Scope
One product file (
_status.py), its controls, and prose corrections in five files. No schema, migration, ledger-writer, retry-writer or root-maintenance change. Pre-existing absorption and live-work-ordering disagreements were deliberately left out of scope by the ruling.