refactor: deterministic state-file key ordering#15
Merged
dhruva-reddy merged 1 commit intomainfrom May 2, 2026
Merged
Conversation
Contributor
Author
This was referenced May 1, 2026
dhruva-reddy
added a commit
that referenced
this pull request
May 1, 2026
## ELI5 **Problem.** Even when you ran a *scoped* push — say `npm run push -- <env> assistants/foo.md` to update one assistant — the engine rewrote the **entire** state file. Any pre-existing drift in unrelated state entries (UUIDs from earlier sessions, untracked local files, etc.) swept into the focused commit. Reviewers couldn't tell from the state-file diff "what did this push actually change?" and the state file became a pile of side effects accumulated across sessions instead of a precise record of intent. **What this fix does.** During a push, the engine tracks which `resourceId`s it actually mutated (a per-section `Set<string>`). At end-of-run, for **scoped pushes only**, it loads the on-disk state fresh, replaces only the touched entries with the in-memory version, and leaves everything else alone. Full pushes (no scope) still write wholesale (existing behavior). Credentials are always replaced because bootstrap pull populates them every push regardless. This depends on Stack F's `ResourceState` because we need per-entry metadata to distinguish "stale" from "just-not-touched." **Outcome you'll notice.** A one-file `npm run push` produces a one-file diff in the state file — same scope as the resource change. Reviewers can read the state diff and tell "this push updated assistant `foo`, here's its new hash" cleanly. Pre-existing drift elsewhere in state stays where it is until you explicitly address it. --- When push is scoped to specific paths, only update state entries for the resources actually touched. A surgical push of two files used to rewrite the entire state file, sweeping in pre-existing drift from earlier pushes (improvements.md #15) and producing noisy diffs that hide the actual scope of the change. Files: - src/state-merge.ts (NEW): mergeScoped(disk, inMemory, touched). For each section, replace only touched.X resourceIds with the in-memory version; leave the rest of disk's section as-is. Credentials are always replaced wholesale (bootstrap pull populates them on every push). Pure data, no I/O — safe to test directly. - src/push.ts: TouchedSets tracker. Each upsertState call site records the resourceId. End-of-run, partial pushes call mergeScoped(loadState(), state, touched) before saveState; full pushes save wholesale (existing behavior). - tests/state-merge.test.ts: replace-only-touched, leave-untouched, drift in untouched stays, credentials always replaced. Closes improvements.md #15. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
dhruva-reddy
force-pushed
the
dhruva-reddy/refactor/state-file-key-order
branch
from
898200a to
0f35c9e
Compare
dhruva-reddy
force-pushed
the
dhruva-reddy/docs/improvements-log
branch
from
6cd7e2e to
28aa4c9
Compare
adhamvapi
approved these changes
May 1, 2026
dhruva-reddy
changed the base branch from
dhruva-reddy/docs/improvements-log
to
graphite-base/15
dhruva-reddy
force-pushed
the
dhruva-reddy/refactor/state-file-key-order
branch
from
0f35c9e to
6430703
Compare
dhruva-reddy
force-pushed
the
graphite-base/15
branch
from
28aa4c9 to
c7aa30b
Compare
## ELI5 **Problem.** Every push rewrites `.vapi-state.<env>.json`. JavaScript's `JSON.stringify` keeps whatever order keys happened to land in — and state sections get rebuilt from multiple sources (push, pull, bootstrap) with unpredictable insertion order. Result: about half of every state diff is just lines moving up and down without any actual change. Reviewers stopped reading state diffs because they were mostly noise, which defeats the point of versioning the file. **What this fix does.** Adds a `sortedKeysReplacer` that runs during `JSON.stringify` and emits object keys alphabetically at every nesting level. Arrays stay in their original order (squad member ordering, tool destination priority, etc. are semantic). State writes go through this replacer. **Outcome you'll notice.** The first push after this lands produces a **big one-time diff** of pure reordering across every customer. That's the cost of landing the fix — please don't read the first state diff post-merge, it's churn. Every diff after that shows only real changes: new UUIDs, removed entries, hashes changing. Reviewing state files becomes useful again. --- JS's JSON.stringify honors insertion order. State sections get rebuilt from multiple sources (push, pull, bootstrap) with unpredictable insertion order, so ~half of every state-file diff is pure reorderings that hide the real changes. - src/state-serialize.ts (NEW): sortedKeysReplacer (recursive alphabetical key sort, arrays untouched) + canonicalize (also drops null/undefined leaves; reused by Stack F/G). Kept config-free so tests can import without triggering config.ts's CLI parser. - src/state.ts: saveState now passes sortedKeysReplacer to JSON.stringify. Atomic-write pattern preserved. - tests/state-key-order.test.ts: pin byte-identical serialization across insertion orders, recursion, array preservation, primitive handling, idempotence. Closes improvements.md #17. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
dhruva-reddy
force-pushed
the
dhruva-reddy/refactor/state-file-key-order
branch
from
6430703 to
2fc1864
Compare
dhruva-reddy
added a commit
that referenced
this pull request
May 2, 2026
## ELI5 **Problem.** Even when you ran a *scoped* push — say `npm run push -- <env> assistants/foo.md` to update one assistant — the engine rewrote the **entire** state file. Any pre-existing drift in unrelated state entries (UUIDs from earlier sessions, untracked local files, etc.) swept into the focused commit. Reviewers couldn't tell from the state-file diff "what did this push actually change?" and the state file became a pile of side effects accumulated across sessions instead of a precise record of intent. **What this fix does.** During a push, the engine tracks which `resourceId`s it actually mutated (a per-section `Set<string>`). At end-of-run, for **scoped pushes only**, it loads the on-disk state fresh, replaces only the touched entries with the in-memory version, and leaves everything else alone. Full pushes (no scope) still write wholesale (existing behavior). Credentials are always replaced because bootstrap pull populates them every push regardless. This depends on Stack F's `ResourceState` because we need per-entry metadata to distinguish "stale" from "just-not-touched." **Outcome you'll notice.** A one-file `npm run push` produces a one-file diff in the state file — same scope as the resource change. Reviewers can read the state diff and tell "this push updated assistant `foo`, here's its new hash" cleanly. Pre-existing drift elsewhere in state stays where it is until you explicitly address it. --- When push is scoped to specific paths, only update state entries for the resources actually touched. A surgical push of two files used to rewrite the entire state file, sweeping in pre-existing drift from earlier pushes (improvements.md #15) and producing noisy diffs that hide the actual scope of the change. Files: - src/state-merge.ts (NEW): mergeScoped(disk, inMemory, touched). For each section, replace only touched.X resourceIds with the in-memory version; leave the rest of disk's section as-is. Credentials are always replaced wholesale (bootstrap pull populates them on every push). Pure data, no I/O — safe to test directly. - src/push.ts: TouchedSets tracker. Each upsertState call site records the resourceId. End-of-run, partial pushes call mergeScoped(loadState(), state, touched) before saveState; full pushes save wholesale (existing behavior). - tests/state-merge.test.ts: replace-only-touched, leave-untouched, drift in untouched stays, credentials always replaced. Closes improvements.md #15. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Contributor
Author
Merge activity
|
dhruva-reddy
added a commit
that referenced
this pull request
May 2, 2026
## ELI5 **Problem.** Even when you ran a *scoped* push — say `npm run push -- <env> assistants/foo.md` to update one assistant — the engine rewrote the **entire** state file. Any pre-existing drift in unrelated state entries (UUIDs from earlier sessions, untracked local files, etc.) swept into the focused commit. Reviewers couldn't tell from the state-file diff "what did this push actually change?" and the state file became a pile of side effects accumulated across sessions instead of a precise record of intent. **What this fix does.** During a push, the engine tracks which `resourceId`s it actually mutated (a per-section `Set<string>`). At end-of-run, for **scoped pushes only**, it loads the on-disk state fresh, replaces only the touched entries with the in-memory version, and leaves everything else alone. Full pushes (no scope) still write wholesale (existing behavior). Credentials are always replaced because bootstrap pull populates them every push regardless. This depends on Stack F's `ResourceState` because we need per-entry metadata to distinguish "stale" from "just-not-touched." **Outcome you'll notice.** A one-file `npm run push` produces a one-file diff in the state file — same scope as the resource change. Reviewers can read the state diff and tell "this push updated assistant `foo`, here's its new hash" cleanly. Pre-existing drift elsewhere in state stays where it is until you explicitly address it. --- When push is scoped to specific paths, only update state entries for the resources actually touched. A surgical push of two files used to rewrite the entire state file, sweeping in pre-existing drift from earlier pushes (improvements.md #15) and producing noisy diffs that hide the actual scope of the change. Files: - src/state-merge.ts (NEW): mergeScoped(disk, inMemory, touched). For each section, replace only touched.X resourceIds with the in-memory version; leave the rest of disk's section as-is. Credentials are always replaced wholesale (bootstrap pull populates them on every push). Pure data, no I/O — safe to test directly. - src/push.ts: TouchedSets tracker. Each upsertState call site records the resourceId. End-of-run, partial pushes call mergeScoped(loadState(), state, touched) before saveState; full pushes save wholesale (existing behavior). - tests/state-merge.test.ts: replace-only-touched, leave-untouched, drift in untouched stays, credentials always replaced. Closes improvements.md #15. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
dhruva-reddy
added a commit
that referenced
this pull request
May 2, 2026
## ELI5 **Problem.** Even when you ran a *scoped* push — say `npm run push -- <env> assistants/foo.md` to update one assistant — the engine rewrote the **entire** state file. Any pre-existing drift in unrelated state entries (UUIDs from earlier sessions, untracked local files, etc.) swept into the focused commit. Reviewers couldn't tell from the state-file diff "what did this push actually change?" and the state file became a pile of side effects accumulated across sessions instead of a precise record of intent. **What this fix does.** During a push, the engine tracks which `resourceId`s it actually mutated (a per-section `Set<string>`). At end-of-run, for **scoped pushes only**, it loads the on-disk state fresh, replaces only the touched entries with the in-memory version, and leaves everything else alone. Full pushes (no scope) still write wholesale (existing behavior). Credentials are always replaced because bootstrap pull populates them every push regardless. This depends on Stack F's `ResourceState` because we need per-entry metadata to distinguish "stale" from "just-not-touched." **Outcome you'll notice.** A one-file `npm run push` produces a one-file diff in the state file — same scope as the resource change. Reviewers can read the state diff and tell "this push updated assistant `foo`, here's its new hash" cleanly. Pre-existing drift elsewhere in state stays where it is until you explicitly address it. --- When push is scoped to specific paths, only update state entries for the resources actually touched. A surgical push of two files used to rewrite the entire state file, sweeping in pre-existing drift from earlier pushes (improvements.md #15) and producing noisy diffs that hide the actual scope of the change. Files: - src/state-merge.ts (NEW): mergeScoped(disk, inMemory, touched). For each section, replace only touched.X resourceIds with the in-memory version; leave the rest of disk's section as-is. Credentials are always replaced wholesale (bootstrap pull populates them on every push). Pure data, no I/O — safe to test directly. - src/push.ts: TouchedSets tracker. Each upsertState call site records the resourceId. End-of-run, partial pushes call mergeScoped(loadState(), state, touched) before saveState; full pushes save wholesale (existing behavior). - tests/state-merge.test.ts: replace-only-touched, leave-untouched, drift in untouched stays, credentials always replaced. Closes improvements.md #15. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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
2 participants
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.
ELI5
Problem. Every push rewrites
.vapi-state.<env>.json. JavaScript'sJSON.stringifykeeps whatever order keys happened to land in — andstate sections get rebuilt from multiple sources (push, pull, bootstrap)
with unpredictable insertion order. Result: about half of every state
diff is just lines moving up and down without any actual change.
Reviewers stopped reading state diffs because they were mostly noise,
which defeats the point of versioning the file.
What this fix does. Adds a
sortedKeysReplacerthat runs duringJSON.stringifyand emits object keys alphabetically at every nestinglevel. Arrays stay in their original order (squad member ordering, tool
destination priority, etc. are semantic). State writes go through this
replacer.
Outcome you'll notice. The first push after this lands produces a
big one-time diff of pure reordering across every customer. That's
the cost of landing the fix — please don't read the first state diff
post-merge, it's churn. Every diff after that shows only real changes:
new UUIDs, removed entries, hashes changing. Reviewing state files
becomes useful again.
JS's JSON.stringify honors insertion order. State sections get rebuilt
from multiple sources (push, pull, bootstrap) with unpredictable
insertion order, so ~half of every state-file diff is pure reorderings
that hide the real changes.
key sort, arrays untouched) + canonicalize (also drops null/undefined
leaves; reused by Stack F/G). Kept config-free so tests can import
without triggering config.ts's CLI parser.
Atomic-write pattern preserved.
insertion orders, recursion, array preservation, primitive handling,
idempotence.
Closes improvements.md #17.
🤖 Generated with Claude Code