The working set stops being a blob every submit overwrites (v0.5.0) - #109
Merged
Conversation
firstproof-c4 was the symptom: a stale "NEXT: run the sweep" directive, a note about attempt 7's formatting failure, and a salvage fragment from a 600-second timeout, all pinned in the frontier with no way to shift them except an admin editing the field by hand. The cause is three properties of how state was stored, none of them specific to that conjecture. REPLACEMENT. `UPDATE targets SET state = $2` — last writer wins over the whole blob. To add one key an agent had to reconstruct the entire working set, so any submit could destroy what earlier work established, and any key an agent didn't recognise got copied forward forever: dropping it was indistinguishable from destroying a real result. State now merges per key. Omission changes nothing; `$retract: [...]` is the only way to remove a key, which makes retiring a dead end a deliberate, attributable act an agent can perform — no admin required. LOSS. The 64 KB cap truncated by keeping the tail, so the first thing dropped as state grew was the accumulated head — the established facts. Facts now live in target_facts (migration 015): append-only rows, deduped by claim so replays and independent rediscovery collapse to one, each carrying the contribution that established it, retracted rather than deleted when superseded, and outside the blob cap entirely. STALENESS. "NEXT: …" was hand-written prose describing work, so it was stale the moment that work landed. next_steps is now DERIVED from the task graph on every read — claimable tasks cheapest-first, counts in flight and expired, and a stalled flag. Nothing to keep up to date because nothing is stored. The expired count also surfaces a systematic-failure signal (c4's timeout pile) that no prose field would ever have shown. Two follow-on fixes fell out. A truncation marker is a tombstone the platform wrote, not a working set an agent built, so the next real update replaces it rather than merging — otherwise `truncated`/`note`/`tail` would pin themselves in place permanently, a fresh species of the exact debris this removes. And the timeout salvage stops spreading the prior state back over itself to protect it: that worked, but re-published every key the run never touched, which is literally how c4's timeout_salvage outlived its attempt. It now sends only its own key and lets the server merge. The executor's output schema tells agents about `facts` and `$retract`, so the mechanism doesn't ship inert. Old runners stay compatible: they send the whole blob, and merging that is idempotent. Server-side merge deploys on merge to main, ahead of any CLI release that depends on it. v0.5.0: src/executor.ts is in the CLI bundle, so this changes the published runner, not just the Worker. 587 tests (14 new). Verified end to end on a c4-shaped target: legacy blob preserved, a fact added without re-sending the blob, the debris retracted by an agent with no admin involved, and the fact surviving a subsequent timeout salvage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQgFEPRY4W74D4eqawQ4A6
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
This was referenced Aug 2, 2026
Barneyjm
added a commit
that referenced
this pull request
Aug 2, 2026
Four parallel review agents (reuse / simplification / efficiency / altitude) over the #109–#111 surface. Applied: LEAK. deriveNextSteps filtered only status='open' and was missing the two exclusions listAvailableTasks documents — onboarding_dev_id IS NULL and sensitivity = 'public'. getTargetProgress is unauthenticated, and the onboarding target is public and slugged, so next_steps.claimable would advertise per-dev onboarding tasks nobody else can claim, plus non-public tasks on public conjectures. They stay in the status counts; they never reach `claimable`. THE TOMBSTONE WAS THE WRONG SHAPE. boundedStateUpdate kept the byte TAIL of the serialized state under {truncated, note, tail} — not parseable JSON, so unreadable to the next agent, and an object of alien keys that then forced mergeStateUpdate to duck-type the marker so a later write wouldn't inherit it. Bounding is now key-aware: drop whole keys largest-first, name them in `_dropped`, and the stored value stays a real working set at every size. The isTombstone special case goes with its cause. Byte-preserving only mattered while established results lived in the blob; they live in target_facts now. `_dropped` is platform-owned and cleared on every merge, so one overflow cannot brand a working set forever. THE FIXED PATTERN HAD AN UNFIXED TWIN. salvageCrashedRun still spread the prior state back over itself, with a comment claiming it matched the timeout salvage — which #110 had changed. Both now send only their own key. EFFICIENCY. The per-fact INSERT loop ran one round trip per claim while holding FOR UPDATE on the targets row, with nothing capping how many claims an agent may send; now one unnest insert. getTargetProgress's three independent reads run concurrently. checkout folds the pending-decomposition predicate into the task SELECT it already runs, instead of a second round trip inside the money-path transaction. Migration 016 adds the two missing indexes: tasks(target_id, status) for the per-page-view scan, and a partial expression index for the review_of JSONB probe. TIDYING. deriveNextSteps counts in one pass, not five filters. The dead synthesizeWorkUnitSummary alias is gone. ParsedStateUpdate.state is typed `unknown`, which is what it always was. Skipped, with reasons in the reply: hydrating facts into checkout (real gap, own PR), facts/$retract as envelope siblings rather than reserved keys (interface decision), a real review_of column (larger migration), one shared isPlainObject (five files outside this diff). 595 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQgFEPRY4W74D4eqawQ4A6
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.
C4 is the symptom. This is the cause.
That target carries a stale
NEXT: run the sweepdirective, a note about attempt 7's formatting failure, and a salvage fragment from a 600-second timeout — all pinned in the frontier with no way to shift them except an admin editing the field by hand. None of the three causes is specific to C4.1. Replacement → merge
UPDATE targets SET state = $2. Last writer wins over the whole blob, so to add one key an agent had to reconstruct the entire working set. Two consequences: any submit could destroy what earlier work established, and any key an agent didn't recognise got copied forward forever — dropping it was indistinguishable from destroying a real result.State now merges per key. Omission changes nothing.
$retract: ["key"]is the only way to remove one, which turns retiring a dead end into a deliberate, attributable act an agent can perform itself. That is the part that answers "I can't be fixing fields for the rest of our lives."2. Loss → append-only facts
The 64 KB cap truncated by keeping the tail, so the first thing dropped as state grew was the accumulated head: the established facts.
Facts move to
target_facts(migration 015) — append-only rows, deduped by claim so replays and independent rediscovery collapse to one, each carrying the contribution that established it, retracted rather than deleted when superseded (same principle as the ledger), and outside the blob cap entirely.3. Staleness → derived next steps
NEXT: …was hand-written prose describing work, so it went stale the moment that work landed.next_stepsis now derived from the task graph on every read: claimable tasks cheapest-first, counts in flight and expired, and astalledflag. Nothing to keep up to date because nothing is stored.The
expiredcount is a bonus — it surfaces the systematic-failure signal (C4's pile of 15 timed-out tasks) that no prose field would ever have shown.Two bugs this surfaced
{truncated, note, tail}marker is written by the platform, not built by an agent, so the next real update replaces it rather than merging. Merging would have pinned those three keys permanently — a fresh species of the exact debris this change removes. Caught by an existing test.salvageTimedOutRunspread the prior state back over itself to avoid clobbering it. That worked, but re-published every key the run never touched — literally how C4'stimeout_salvageoutlived its attempt. It now sends only its own key and lets the server merge.Compatibility
The executor's output schema now documents
factsand$retract, so the mechanism doesn't ship inert. Old runners stay compatible: they send the whole blob, and merging that is idempotent. Server-side merge deploys on merge to main, ahead of any CLI release that depends on it. Existing free-form states are untouched and keep rendering.v0.5.0 — unlike #107/#108,
src/executor.tsis in the CLI bundle (verified: the new schema text appears indist/givework.mjs), so this changes the published runner and not just the Worker.Testing
587 tests, 14 new. Verified end to end on a C4-shaped target:
Not in this PR
facts/next_stepson the conjecture page. Server-side only here; a migration plus a submit-path change deserves review without UI noise.🤖 Generated with Claude Code
https://claude.ai/code/session_01QQgFEPRY4W74D4eqawQ4A6