Skip to content

Load a save once: tabs stop loading one by one - #80

Merged
dh0er merged 17 commits into
mainfrom
claude/save-editor-tab-loading-8d3d8c
Aug 13, 2026
Merged

Load a save once: tabs stop loading one by one#80
dh0er merged 17 commits into
mainfrom
claude/save-editor-tab-loading-8d3d8c

Conversation

@dh0er

@dh0er dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner

Opening a savegame and then walking its tabs cost roughly twelve seconds of core
work on a real save (2.5 MB file, 119 MB decoded payload, 1.4M properties), and
every repeat paid the same again. Each tab loaded on first paint, so every first
click sat on a spinner.

Measured on a real save, min of three runs:

step before after
inspect_save 3883 ms 2.0 ms¹
world clock 1073 ms 2.0 ms
character index 513 ms 2.0 ms
hero attributes 891 ms 1.8 ms
skills 700 ms 1.8 ms
NPC list 257 ms 1.9 ms
quests / glossary / tutorials 149 / 228 / 154 ms ~2 ms
knowledge / events 151 / 47 ms ~2 ms
property browser 603 ms 2.0 ms
backups 130 ms 51 ms
total 8785 ms 78 ms

¹ a repeat. The first open of a save is 7.7 s → 1.7 s; that is the part that
genuinely has to be computed.

What changed

The typed search allocated three Strings per property. For each of ~1.4M
properties it built a path segment, a formatted value, and — once per query term
— a lower-cased copy of the whole display path, just to test a substring. The
path now borrows property names out of the tree and owns only the
[index]/{mapKey} segments it has to format, a lower-cased twin of the
display path is kept in lockstep, and the path and value are built only for a
property that reaches the result page.

inspect_save ran eleven independent read-only passes back to back — the
FString scan, the typed parse, and one traversal each for the inventory, armor,
slot integrity, progression, NPC, faction, skill and glossary blocks. They share
no state, so they now run on scoped threads in two stages and the load costs the
longest pass instead of their sum. The inventory summary is split so its byte
scan no longer waits for typed-tree traversals it does not depend on.

private.skills.list went around the parsed-root cache — the one read
command that did. It copied the whole decoded payload out of the byte cache and
re-parsed it.

No command result was cached. Read commands now memoize their response under
the request plus a content fingerprint of every file the answer depends on; for
inspect_save that includes the sibling PersistentDataList.sav, which carries
the slot's profile assignment. list_backups and scan_save_dir are
deliberately excluded — they describe a folder, which changes without any save
changing. A repeat read costs 2 ms: reading and hashing the file.

list_backups read and hashed every backup file serially. A folder with a
hundred-odd backups made that a tenth of a second on every save selection; the
candidates are now described in parallel. Disk-bound, so it does not scale with
cores.

The editor prefetches every tab. Once an inspection lands, a background
warm-up issues exactly the queries the panels will issue, ordered by how soon
the user can reach them: overview, then Characters, World, and the property
browser. It never touches the loading overlay or reports an error, and it stops
the moment a newer load or a write supersedes it, so it cannot make the user's
own request wait behind the rest of the warm-up. The panels' page sizes moved to
EditorPageSize because the cache holds one response per exact request — a
panel that quietly picked its own size would be warmed with an answer it never
asks for.

Verification

  • Byte-identical output. Seventeen commands over a frozen copy of a real
    save, including the preview and public-only inspects — 2.2 MB of responses,
    unchanged against the previous core. The typed search was separately diffed
    over eight queries including deep pages and the empty query (3.1 MB).
  • New integration tests pin the response cache against serving a stale read
    after a write, after an out-of-band file replacement (a cloud sync, the game
    saving over the slot), and for a directory listing that must not be cached.
  • New tests cover the prefetch: what it warms, that its page sizes match the
    panels', that it runs once per inspection, that it never raises the loading
    overlay, and that a newer load stops it.
  • 412 Rust tests, 546 Flutter tests, flutter analyze clean.

Two tests in player_events_hero_wiring_test forbade every progression query
while the player had no hero id. Their subject — and their own comment — is the
events query, which is still forbidden and still guarded; the other sections now
legitimately load in the background.

🤖 Generated with Claude Code


Note

Medium Risk
Large changes to core caching, save inspection parallelism, and background prefetch concurrency; correctness depends on fingerprint invalidation and supersede logic, with integration tests added but real-save edge cases (external file replacement, sidecars) matter for stale reads.

Overview
Save editor tabs feel instant after open because the Flutter layer now background-prefetches every panel’s core queries as soon as a save finishes loading (prefetchTabData on state changes), using shared EditorPageSize so warm-up requests match real panel pagination. Prefetch does not show loading UI, yields when a newer load/write supersedes it, walks multi-page quest/story/NPC lists, and ends with warm_save so the decoded tree is ready before NPC detail reads.

The Rust core (gore-save) adds a content-fingerprinted response cache for read commands (invalidated on write; directory listings excluded), a warm_save command to re-seed decode/parse caches when inspect_save is served from cache, parallel inspect_save summary passes and parallel backup listing reads, typed property search allocation reductions, and private.skills.list now uses the shared parsed-root cache instead of re-parsing.

Reviewed by Cursor Bugbot for commit 917a6d7. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread apps/save-editor/lib/features/editor/domain/editor_notifier.dart

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7e3d2e9cf3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/gore-save/src/lib.rs
@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b237794. Configure here.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b237794fb7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/gore-save/src/lib.rs
@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 758a7f3. Configure here.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 758a7f3bfa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/save-editor/lib/features/editor/domain/editor_notifier.dart Outdated
@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 66d1fcd. Configure here.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: 66d1fcd312

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@cursor review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 66d1fcd312

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/gore-save/src/properties.rs Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 66d1fcd. Configure here.

@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 72c29da. Configure here.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 72c29dae04

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/save-editor/lib/features/editor/domain/editor_notifier.dart Outdated
@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@cursor review

Comment thread apps/save-editor/lib/features/editor/domain/editor_notifier.dart Outdated
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 3a2f02338f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 28d844e. Configure here.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 28d844e9e1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/gore-save/src/lib.rs
@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit f7b77e5. Configure here.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: f7b77e5a5f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@dh0er

dh0er commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@cursor review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: f7b77e5a5f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit f7b77e5. Configure here.

dh0er and others added 17 commits August 13, 2026 19:37
The property search walked the whole tree building, for every one of the
~1.4M properties in a real save, a String for the path segment, a String
for the formatted value, and — once per query term — a lower-cased copy
of the whole display path just to test a substring.

Carry the path in a struct that borrows property names out of the tree and
owns only the `[index]`/`{mapKey}` segments it has to format, keep a
lower-cased twin of the display path in lockstep with it, and build the
path and value only for a property that actually reaches the result page.

Verified byte-identical against the previous walk over a real save: eight
queries, including deep pages and the empty query, 3.1 MB of results.

Hero attributes 871 ms -> 277 ms, the world clock 844 ms -> 274 ms.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Opening a save and then walking its tabs cost around twelve seconds of
core work on a real save, and every repeat paid it again. Four things:

- `inspect_save` ran eleven independent read-only passes back to back:
  the FString scan, the typed parse, and one traversal each for the
  inventory, armor, slot integrity, progression, NPC, faction, skill and
  glossary blocks. They share no state, so they now run on scoped threads
  in two stages, and the load costs the longest pass instead of their sum.
  The inventory summary is split so its byte scan no longer waits for the
  typed-tree traversals it does not depend on. 7.7 s -> 1.7 s.

- `private.skills.list` was the one read command that went around the
  parsed-root cache: it copied the whole decoded payload out of the byte
  cache and re-parsed it. 700 ms -> 60 ms.

- No command result was cached, so re-opening a save or returning to a tab
  recomputed everything. Read commands now memoize their response under
  the request plus a content fingerprint of every file the answer depends
  on — for `inspect_save` that includes the sibling PersistentDataList.sav,
  which carries the slot's profile. Directory listings (`list_backups`,
  `scan_save_dir`) are deliberately excluded: they describe a folder, which
  changes without any save changing. A repeat read is now 2 ms, the cost of
  reading and hashing the file.

- `list_backups` read and hashed every backup file one after another; a
  folder with a hundred-odd backups made that a tenth of a second on every
  save selection. The candidates are now described in parallel. 130 ms ->
  50 ms (disk-bound, so this does not scale with cores).

Verified byte-identical against the previous core: seventeen commands over
a real save, including the preview and public-only inspects, 2.2 MB of
responses. New integration tests pin the cache against serving a stale read
after a write, after an out-of-band file replacement, and for a directory
listing that must not be cached at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…erview

Each tab loaded its data on first paint, so the first click on Characters,
World or All data sat on a spinner for as long as the query took. The core
now answers a repeated read from a content-keyed cache, which makes it
worth asking ahead of time.

Once an inspection lands, the editor page starts a background warm-up that
issues exactly the queries the panels will issue, ordered by how soon the
user can reach them: the overview first, then Characters, World, and the
property browser. It never touches the loading overlay or reports an error,
and it stops the moment a newer load or a write supersedes it, so it can
never make the user's own request wait behind the rest of the warm-up.

The panels' page sizes move to `EditorPageSize` because the cache holds one
response per exact request: a panel that quietly picked its own size would
be warmed with an answer it never asks for.

Two tests in player_events_hero_wiring_test forbade *every* progression
query while the player had no hero id. Their subject — and their own
comment — is the events query, which is still forbidden and still guarded;
the other sections now legitimately load in the background.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ackups

Two reports from the PR's automated reviewers, both real.

Cursor: the warm-up never ran. `_inspect` publishes the inspection while it
is still fetching the backup list, so the page's first state-change call
arrived with the editor still loading. It claimed the inspection, every
warm-up step bailed out on that flag, and the call that arrived once loading
ended found the inspection already claimed and skipped it. Opening a save
warmed nothing. Wait for the flag instead of spending the trigger on it:
clearing it is itself a state change, so the page comes back.

Codex: `private.npc.position` reports the recorded placement undo, which
lives in a sidecar beside the save rather than inside it, and the response
cache fingerprinted only the save. Restoring a backup puts back
byte-identical save bytes alongside that backup's placement notes, which
would then be served from the pre-restore cache entry. The two commands with
such a dependency now declare it in one place, `response_companion_files`.

Both are covered by tests that fail without the fix: opening a save driven
only by state changes (as the page drives it) must warm the tabs, and a
placement note recorded or cleared without touching the save must change what
`private.npc.position` answers.

One existing widget test forbade every events query while an orphan was
selected. Its subject is the orphan, which has no GlobalId to ask with; the
player's own events now legitimately load in the background, so it counts the
queries the orphan selection itself caused.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…describe

Codex on the PR: the response cache took its content fingerprint, then let the
command read the same files again for itself. If a file changed between those
two reads — the game saving over the slot, a cloud sync — the answer computed
from the new bytes was stored under the old bytes' fingerprint. That entry is
not stale for a moment; it answers every later read of the ORIGINAL bytes, so
restoring them serves the wrong data for as long as the entry lives.

Re-derive the fingerprint after the command and keep the response only when it
still matches, so a stored entry always describes the content its key names.
Costs one extra read and hash on a cache MISS, next to the hundreds of
milliseconds the miss itself costs; a hit is unaffected.

A cache hit needs no such re-check and does not get one: matching a fingerprint
means the files hold byte-identical content to what the entry was built from,
whatever happened in between.

Not covered by a test. The window is between two reads that sit microseconds
apart, so a test cannot land inside it reliably — a timing-based attempt passed
just as well without the fix, which makes it worse than no test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex on the PR: `loadAllNpcActors` memoizes its roster for the lifetime of
one inspection, and the warm-up was calling it, so the memo was filled at load
time rather than when a panel first needed it. A save replaced in between — the
game, a cloud sync — would then leave the first NPC panel showing a roster
fetched from bytes no longer on disk, where before the fix it would have
fetched against the current file.

The memo itself predates this branch and is deliberate: the whole editor is
pinned to one inspection, and every other panel reloads only when a new one
lands. What did not belong there was the warm-up reaching into it. Page the
roster into the CORE's cache instead and leave the memo to the panel that
actually uses it: the paging it then repeats is answered from the warm cache,
so the visit stays fast while the roster is derived from the file as of that
moment.

The paging loop moves to `_fetchAllNpcActors`, which both callers share. Only
the memoizing one clears the memo slot on a failed page; the warm-up has no
slot to clear, and clearing it behind a real load in flight would be wrong.

Covered by a test that fails when the warm-up fills the memo.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex on the PR, and it disproves the reasoning I had written into the
comment above the code. The search builds a lower-cased twin of the display
path one segment at a time and matches query terms that went through
`str::to_lowercase`. I lower-cased the segments char by char, arguing that
segment boundaries never move a character into a different word position.
That much is true, but it misses the point: `char::to_lowercase` has no
context to apply, so it maps a word-final Σ to σ where `str::to_lowercase`
maps it to ς. Searching "ΟΣ" normalized the query to "ος" and the path to
"οσ", and the property could not be found by its own name.

Lower-case each segment as a string. ASCII — everything a real save has in
practice — takes a fast path that lower-cases in place, so the per-property
allocation this walk exists to avoid stays avoided; only a non-ASCII segment
allocates, and only that segment.

Covered by a test over a word-final and a word-medial sigma; it fails on the
previous mapping. Typed-search output over a real save is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex on the PR. The warm-up claimed its inspection when it started, and its
steps skip while anything else holds the editor. So an operation that arrives
mid-warm-up — a backup rename or delete bumps the load sequence, a codec check
raises the loading flag — made every remaining step skip, and the identity
check then refused to try again. The tabs those steps would have covered went
back to loading the slow way for the rest of the session, which is exactly the
stall this warm-up exists to remove.

Retire an inspection only after a run that warmed every step. Anything less
leaves the marker unset, so the next state change starts the sequence over;
the steps that did complete come back from the core's cache, so a restart
re-walks them in milliseconds.

The marker can no longer double as the in-flight guard, and the warm-up itself
changes editor state (the character index settles the hero id), so a second
one would otherwise start mid-run. `_prefetchRunning` keeps it to one.

Covered by a test that interrupts a warm-up and asserts the next trigger picks
it up — and that a completed one is still retired.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… be called

Cursor on the PR, against the previous commit's fix. Leaving the inspection
unmarked when a run is cut short only helps if something calls the warm-up
again — and a step still in flight holds `_prefetchRunning` past the moment the
interrupting operation clears the loading flag. The state change that would
have restarted the warm-up therefore bounces off that guard, and by the time
the drained run releases it nothing is left to trigger anything. The tabs it
never reached stayed cold for the session, which is what the previous commit
set out to prevent.

A run that ends without warming everything now re-arms itself. It cannot spin:
a fresh run takes the current load sequence, so it can only be cut short by a
NEW interruption, and the entry guards still decide whether it may start.

The test moves to the ordering that actually exposes this — the first warm-up
step outlives the interruption — and drives the notifier through the page's own
listener, so the restart cannot be attributed to a trigger the test made by
hand. It fails without the re-arm.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex on the PR is right that equal fingerprints before and after a command do
not prove the files held still: a replacement to other bytes and back again
inside the command would pass the check and bind that other answer to the
original content hash.

Left as is, with the reasoning recorded where the check lives rather than a
comment that overstates it. Closing it properly means handing the command the
bytes the key was taken from instead of letting it read for itself — a change
to how every read command receives its save, not to this check, and not one to
make inside a performance branch.

What remains needs a replacement landing in the moment between two reads that
sit microseconds apart AND the original bytes returning before the command
ends, both from outside the editor: a write or a restore performed here drops
the save's cached responses outright.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex on the PR. The quest and story panels fetch their section whole and
filter it in the client, walking pages the core clamps to 1000. The warm-up
asked for one page, so a save past that clamp would open those tabs on a cold
traversal for every page after the first — spinner up, which is the wait this
warm-up exists to remove.

Walk the pages the way the panels do, mirroring their offset arithmetic:
the core caches one response per exact request, so an offset they never ask
for warms nothing. A save inside the clamp costs exactly one request, as
before — the real save measured here holds 472 quests and 470 story entries,
so this changes nothing today and holds when either outgrows the page.

Covered by a test over a section with three pages and one with exactly one;
it fails when only the first page is warmed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cursor on the PR, against the previous commit's paging. A walk checked for
supersede once, before it began, and then kept issuing pages — so a save switch
or a load partway through left it queueing requests ahead of the user's own,
to warm offsets derived from the previous file's total that nothing would ask
for. The quest walk also passed no path, so those later pages queried whichever
save was selected by then.

Check before every page, and pin the quest pages to the file the walk began
against, as the story and NPC walks already were. The NPC walk gets the same
supersede check; a real load passes none, because a panel that asked for the
roster needs all of it rather than a prefix.

`loadProgressionQuests` grows the `path` parameter its story counterpart
already has. Defaulting to the current selection keeps the panels' requests —
and so the cache keys — exactly as they were.

Covered by a test that disturbs the editor from INSIDE the walk, on its first
page. My first attempt disturbed it from outside and proved nothing: the
interruption landed before the walk had even started, so the sequence skipped
it wholesale and the counts matched either way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…it holds

Two from Codex on the PR.

The sibling-uniqueness check was my own regression. Replacing the per-list
name map with a scan per property was right for the handful of entries a
property list normally has, and quadratic for a list that has many. Measured
on one wide list, scanning every time against counting once:

     1,000 siblings    2.4 ms  ->  0.15 ms
     5,000 siblings   52.9 ms  ->  0.46 ms
    20,000 siblings  753.1 ms  ->  3.0 ms
    50,000 siblings    6.1 s   ->  7.5 ms

Keep the scan below a threshold, where it beats building a map the walk would
throw away, and count once above it. A test pins that both sides agree on
which names are unique — a disagreement would either hide editable properties
or, worse, offer a duplicated name as editable and let a write resolve to the
wrong one.

The response cache counted only responses against its byte budget while each
entry also holds its request verbatim, and a request carries caller-supplied
text with no bound of its own. Count everything an entry keeps alive.

Typed-search output over a real save is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex on the PR. The backup listings spread their work across one worker per
logical CPU, and each worker holds a whole file while it hashes and inspects
it — so peak memory was cores x file size, with nothing bounding either.

The severity does not carry over from the report's example: this path passes
`include_private: false`, so it never decompresses, and a G1R save is a couple
of megabytes on disk rather than the 80 MiB assumed there. Against the folder
measured here — 147 backups, largest 2.6 MB — 24 workers meant about 62 MB, not
gigabytes.

The bound is still worth having, and costs almost nothing, because the work is
disk-bound and the curve flattens early (serial 130 ms):

     2 workers   99 ms       8 workers   60 ms
     4 workers   70 ms      12 workers   62 ms

Cap at eight. Past there nothing is left to win, and every further reader is
another whole file held in memory for it. `par_map` takes the cap from its
caller, since the right bound follows from what a worker holds rather than from
how many cores are idle.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex on the PR. The decoded payload and parsed tree live in caches that hold
ONE save each, and `inspect_save` seeds them on the way past — except when its
own response comes from the response cache. That is exactly what happens on
returning to a save opened earlier: the inspection lands in milliseconds while
those caches still hold whichever save was opened in between, and the warm-up
that follows is all cache hits, so nothing puts it right. The first read that
needs the tree then pays for it in front of the user. Measured on a real save,
opening A, switching to B and returning to A:

    the first NPC detail after returning    1.3 s

Per-NPC panels cannot be warmed one by one — a save holds ~1500 NPCs — so what
has to be warmed is the tree itself. `warm_save` asks the core to make a save
the one it holds, and the background warm-up calls it first, ahead of every
step that benefits from it. The same read then costs 4.7 ms; the 1.3 s happens
while the user is still reading the Overview tab. On a normally loaded save the
call is a cache hit and costs a file read and a hash.

Deliberately not response-cached: a stored "warmed" would skip the seeding that
is the whole point of the call. A test pins that, and another pins that warming
moves the reparse off the following read — both fail without the fix.

Both are timing tests, because the property IS timing: every path here returns
the same answer. They compare two measurements from the same run rather than a
fixed budget. The file's tests now also take a mutex, since the caches they
exercise are process-global and were displacing each other under the default
parallel test run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cursor on the PR, against the previous commit. Putting the tree rebuild first
made it the one thing standing between the user and a cached tab: it runs on
the shared core queue, and on a returned-to save it holds that queue for over a
second while every step behind it — and every Characters or World click in that
window — is an answer already sitting in the cache.

Move it to the end. The case where the rebuild costs anything is precisely the
case where all the tab queries are cached, so they cost milliseconds ahead of
it; on a normally loaded save the call is a cache hit wherever it sits. The
tabs the user can click stay instant either way, and the tree is rebuilt behind
them, in time for the first NPC they open.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The merchant panel arrived after this branch did, so the timing harness never
covered it. Both its commands go through the shared parsed-root cache, which is
why they need nothing else: 2.7 ms for the merchant list and 2.3 ms for one
merchant's record on a warm tree, and 50 ms for clicking through ten merchants,
against 1.1 s for the first read when the core holds a different save.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dh0er
dh0er force-pushed the claude/save-editor-tab-loading-8d3d8c branch from f7b77e5 to 917a6d7 Compare August 13, 2026 17:48

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 917a6d7. Configure here.


/// Fetched whole and then filtered/paged in the client: quests, tutorials,
/// story state.
static const fullList = 1000;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Story panel skips shared page size

Low Severity

EditorPageSize.fullList exists so prefetch and panels share one exact request shape for the response cache, and the comment even names story state. Prefetch warms story with that constant, but story_state_panel.dart still hardcodes 1000. Values match today; changing fullList later would warm a limit the story panel never asks for, so the first visit pays the cold multi-page load again.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 917a6d7. Configure here.

@dh0er

dh0er commented Aug 13, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 917a6d749a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@dh0er
dh0er merged commit ac87b02 into main Aug 13, 2026
2 checks passed
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