perf(tools): parse a log once and count methods one way - #85
Merged
Conversation
lukecotter
force-pushed
the
perf/parse-once
branch
from
August 3, 2026 13:39
ff604ba to
efe419a
Compare
lukecotter
force-pushed
the
perf/slim-tool-definitions
branch
from
August 3, 2026 13:39
7c774b2 to
37006f6
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
lcottercertinia
force-pushed
the
perf/slim-tool-definitions
branch
from
August 3, 2026 13:42
37006f6 to
c40b792
Compare
lcottercertinia
force-pushed
the
perf/parse-once
branch
from
August 3, 2026 13:42
efe419a to
0e24dde
Compare
lukecotter
force-pushed
the
perf/parse-once
branch
from
August 3, 2026 14:58
0e24dde to
7a7cb19
Compare
lcottercertinia
force-pushed
the
perf/parse-once
branch
from
August 3, 2026 15:08
7a7cb19 to
8d251c0
Compare
The three analysis tools each repeated the same fs.access / readFile / parse preamble and its own NS_TO_MS constant, so a "summary, then go deeper" flow parsed a large log up to three times. They also disagreed on totalMethods: the summary did not count entry points, so it reported 11 where the other two reported 13. src/tools/apexLogSource.ts is now the one way to get a log. loadApexLog caches the last parse by path, size and modification time. isMethodNode is the one method test and walkLog the one traversal, so the totals agree. No tool, parameter or response field changed, but get_apex_log_summary now reports the same totalMethods as the other two tools.
The cache slot held the finished parse, so it was only filled once the read and parse had completed. Two callers that asked for the same log while the first read was still running both missed the slot and both parsed a file that may be 19 MB. Hold the in-flight promise instead. The slot is filled in the same microtask as the miss, so the second caller cannot slip between the check and the write, and a read or parse that fails clears the slot rather than being served on.
MCP revision 2026-07-28 makes the protocol stateless, which invites the reading that a server may not cache. It governs protocol state — the initialize handshake, the session id — not memoised work, and the log path is already the explicit handle its "Stateful Tools" guidance asks a tool to take. Say so where the cache is declared, so the question is not re-opened.
lcottercertinia
force-pushed
the
perf/parse-once
branch
from
August 4, 2026 08:45
ce07e68 to
69da4c8
Compare
lcottercertinia
previously approved these changes
Aug 4, 2026
Size and modification time alone let a cp -p over the cached path serve the old parse: the copy keeps both. Add the inode, which catches a rename over the path, and the change time, which POSIX moves on any change to the inode and no copy tool can hold back. Nanoseconds rather than milliseconds, so two writes inside one millisecond are still two keys. All of it comes from the fs.stat the cache already makes.
A parsed log holds four to five times the size of the file, so a 200 MB log holds about a gigabyte, and until now it held it until the process exited. An idle timer, reset on every hit, gives it back once the agent has moved on. The timer is unrefed, so it never keeps an idle server alive.
lcottercertinia
previously approved these changes
Aug 4, 2026
CodeQL flagged js/file-system-race, high: the cache stated the path and then read the path. Those are two different files if something replaces the path between the two calls, which would put the fingerprint of the old file against the bytes of the new one. Open the file once, and take both the stat and the read from that handle. A handle holds one inode, so the fingerprint now describes exactly the bytes that were parsed. The handle is closed on every path out, including a cache hit. The fs mock in the tests models a handle as the file at one path, so its stat and read go to the same two mocks as before with the path filled in. No test expectation changes.
lcottercertinia
approved these changes
Aug 4, 2026
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
3 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.
Stacked on #83 — review that first; this PR's diff is the last commit.
Problem
Two defects found while measuring for #83:
fs.access/readFile/parsepreamble was copy-pasted into all three analysis tools, each with its ownNS_TO_MSconstant. A "summary, then go deeper" flow parsed a 19 MB log three times.totalMethodsdisagreed between tools.get_apex_log_summarydid not count entry points (CODE_UNIT_STARTED), so it reported 11 whereanalyze_apex_log_performanceandfind_performance_bottlenecksreported 13.Change
New
src/tools/apexLogSource.tsis the one way the analysis tools get a log:loadApexLogreads and parses, and reuses the last parse when the same file is asked for again and neither its size nor its modification time changed. One slot: the flow works through one log at a time, and a parsed 19 MB log is too big to hold several of.isMethodNodeis the one method test, so the three tools agree ontotalMethods.walkLogis the one traversal.NS_TO_MSmoves toresponseShaping.ts.Effect on responses
No tool, parameter or response field changed. The one value that moves is
get_apex_log_summary.totalMethods, which now matches the other two tools — visible in the two re-recorded goldens (11 → 13, 1 → 2). The value changes, not the payload size, so the token cost table #81 publishes is unaffected.Verification
pnpm run build,pnpm run lintcleanpnpm test— 240 passing in 13 suites (8 new forapexLogSource: cache hit, miss on mtime, size and path, missing file,isMethodNode,walkLog)pnpm run eval— 6/6, with the twoget_apex_log_summarygoldens re-recorded for the fix above