Skip to content

fix(topology): read dot-importing files once per census, not per call site (6-10 s packs -> 0.2-0.4 s) - #651

Merged
golimpio merged 1 commit into
mainfrom
atlas/plan-544-census-cost
Oct 13, 2026
Merged

golimpio merged 1 commit into
mainfrom
atlas/plan-544-census-cost

Conversation

@golimpio

Copy link
Copy Markdown
Contributor

What

This fixes a latency regression from #648 (caller completeness). For every recorded call of a seed's name, the census asked whether the call's file dot-imports a package. SQLite answered that through the import-kind index, scanning every import node in the workspace once per call site. For a common method name, such as Execute (962 sites against 11k imports), that took seconds.

This is how arm R (PLAN-543, real open plumb bugs) found it:

The census runs after the walk and before the test estimate, and it was using up the shared clock.

Fix: the set of dot-importing files is read once per census, by one indexed query that takes about 9 ms, and looked up per site in Go. The census claims don't change.

Measured on arm R's actual requests

These are the exact context_for_task arguments from the D transcripts, replayed in-process on the same index (plumb dfa964f, 2,016 files), warm, 3 reps each:

pack before after run-first line
R05 (#601) 7.5–9.6 s 0.36 s missing → present
R08 (#600) 6.4–10.1 s 0.23 s missing → present
R03 (#599) 1.2 s 0.25 s present
R01 (#642, control) 0.15 s 0.23 s present (kept)

A CPU profile before the fix put 91% in Store.OpenCallSites.

Tests

  • TestOpenCallSites_CostDoesNotScaleWithImportsPerSite: 2,000 calls of one name over 20,000 imports must finish within 1.5 s. The fixed query takes about 0.29 s.
    • Positive control: with the per-site probe restored by hand, it fails at 14.9 s.
    • The bound is about 50× wider than the fixed query's time, so it measures the query's shape, not the machine.
  • TestOpenCallSites unchanged: the dot-import flag still drives the bare-call rule, and feat(context_for_task): say when a seed's callers are complete (per-call resolver outcome, schema 8) #648's mutation run killed the "flag lost" mutant.
  • ./internal/topology/... and ./internal/tools both pass.

Why #648 missed it

#648 was verified for correctness (type-checker oracle, mutation tests, container verify-full) but not for latency on the real repository. The gate-v1 latency harness had run on the collector before the census existed. Lesson recorded: a change to context_for_task's collection path gets a replay-latency check against real requests before merge.

🤖 Generated with Claude Code

… site

The caller census (#648) asked, for every recorded call of a seed's name,
whether its file dot-imports a package. The planner answered through the
import-kind index, scanning every import node of the workspace per site, so a
common method name (Execute: 962 sites, 11k imports) took seconds. On arm R's
real tasks a pack took 6-10 s, ran past the 2 s graph deadline and lost its
affected-tests section and run-first line. The set is now read once.

Replayed in-process on the same index (plumb dfa964f): R05 7.5-9.6 s -> 0.36 s,
R08 6-10 s -> 0.23 s, R03 1.2 s -> 0.25 s; all four Go packs carry the run-first
line (R01 control kept). Census claims unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Plumb-Session: warm-cobra

@atlas-from-plumb atlas-from-plumb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved under the owner's standing delegation. The bar is met on head 73096a1:

  • Independent review (scarlet-badger): PASS. Mutation run in its own worktree: DotImport wiring forced false, KILLED; set query on the wrong name, KILLED. Timing test checked under -race at 285 ms against the 1.5 s bound.
  • Positive control: with the per-site probe restored, the cost test fails at 14.9 s.
  • CI 9/9 on 73096a1.
  • make verify-full-container: exit 0, 61 ok, at most 1 daemon, Revision 73096a1.
  • Up to date with main 995915c.

@golimpio
golimpio merged commit 24f8b65 into main Oct 13, 2026
9 checks passed
golimpio pushed a commit that referenced this pull request Oct 14, 2026
…ed time

#651's comments named PLAN-544, a number since given to an unrelated card.
The cost test now logs the census time beside its 1.5 s bound, so a CI
slowdown reads as a shrinking margin before it reddens (review NIT on #651).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Plumb-Session: scarlet-badger
atlas-from-plumb added a commit that referenced this pull request Oct 14, 2026
…(flaky watcher test) (#662)

* fix(config): reload a global config written before the watch started (PLAN-544)

The daemon loads config.toml, then starts the watcher. A write in between
raised its events before any watch existed, and the stat tick compares
against the inode it attached to, so nothing reloaded until the next edit.
The same window is why TestGlobalConfigWatcher_DeleteKeepsLoadedConfig (and
siblings that Save right after `go Run`) timed out at 10 s on loaded CI
runners: the Save beat the first attach.

Run now arms one debounced reload when its first attach finds the file, as
onError already does after a blind recreate. The new test writes the file
before Run, which makes the window certain; it fails on the old code.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Plumb-Session: scarlet-badger

* chore(topology): cite PLAN-542 for the census cost and log its measured time

#651's comments named PLAN-544, a number since given to an unrelated card.
The cost test now logs the census time beside its 1.5 s bound, so a CI
slowdown reads as a shrinking margin before it reddens (review NIT on #651).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Plumb-Session: scarlet-badger

* fix(config): start the global watcher after the topology reconciler subscribes

Review findings on the start reload:
- The watcher started before store.Subscribe(topoPool.Reconcile); a start
  reload landing before that subscription updated the store but never the
  pools built from the first load. It now starts after the subscriber.
- Arm the debounce timer unconditionally: with no file the settle's attach
  fails and nothing reloads, so the armed flag was unobservable.
- The new test fails with Run's own error if Run ends early, and waits 10 s
  rather than 2.5 s so a loaded runner cannot fail a correct watcher.
- The census Logf comment and the CHANGELOG wording now claim only what holds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Plumb-Session: scarlet-badger

* docs(changelog): give each watcher test its own timeout in the PLAN-544 entry

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Plumb-Session: scarlet-badger

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
golimpio added a commit that referenced this pull request Oct 14, 2026
…e packs lost their affected tests) (#663)

* fix(context_for_task): ask each file's sensitivity once per call

The walk asks whether every node it reaches sits in a sensitive file: 641 to
2,023 times per pack over 109 to 242 files on arm R's real requests. The
daemon's decision (connSession.changeSensitive) resolves the path's project on
disk each time (historyRootFor -> workspacePool.Detect -> filepath.Glob for
markers), so on the live daemon a pack overran its 2 s graph deadline even on a
warm, fully resolved index and lost its affected-tests section and run-first
line (PLAN-462 arm R v2: 3 of 4 Go packs). The in-process replays that cleared
#651 wired no sensitive decision, so they could not see it. The answer is now
remembered per path for the one call.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Plumb-Session: warm-cobra

* test(context_for_task): pin that every node of a sensitive file stays withheld under the memo

Review S1 (scarlet-badger): a memo returning the wrong remembered answer leaked
only the second and later nodes of a file, which the control did not check. The
test now requires every cart/cart.go node withheld (at least 2 reached) and none
of its body text in the pack. The sensitiveFor comment says which asks bypass
the memo (review NIT).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Plumb-Session: warm-cobra

---------

Co-authored-by: Atlas from Plumb <atlas@getplumb.sh>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
atlas-from-plumb added a commit that referenced this pull request Oct 14, 2026
)

* docs(site): name context_for_task in the Map card, as experimental

The site counted context_for_task in "Sixty structured tools" and in Map ×9
but never named it. The Map card now lists it and says, in one sentence
backed by the 0.24.1 CHANGELOG entries, what the pack carries: the seeds'
code, callers and related declarations, the tests that reach them (#647),
whether a Go caller list is complete (#648/#651), and a note while the
index is still on its first pass (#668).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Plumb-Session: forest-ridge

* docs(site): keep "in milliseconds" off context_for_task; state its 2 s deadline

Review of #669: the new sentence followed "in milliseconds" and could read as
covered by it, but the pack's graph walk runs under a 2 s deadline
(docs/tools.md § context_for_task). Name it as a separate call, and say so.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Plumb-Session: forest-ridge

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@golimpio
golimpio deleted the atlas/plan-544-census-cost branch October 15, 2026 00:07
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.

2 participants