feat(operations)!: fold repeats into one row by default - #133
Merged
Conversation
Replace @modelcontextprotocol/sdk with @modelcontextprotocol/server and @modelcontextprotocol/client, and serve stdio through the serveStdio factory. The factory runs once per connection, after the opening exchange chooses the protocol era, so the process-level signal handlers and the error callback move out of the ApexLogServer constructor into a new exported runStdioServer. legacy: "serve" is the SDK default and is stated explicitly, because dropping the 2025-era clients would be a second breaking change. The SDK no longer emits execution.taskSupport on a tool definition, so every definition costs 9 tokens less. No response changed. Closes certinia#103
ApexLogServer was a constructor plus one private registerTools, and the SDK v2 factory made its server field public just to reach it through a throwaway instance. createApexLogServer returns the McpServer directly. classificationCache moves to module scope. The factory builds one server per connection, so as an instance field the cache no longer had the process lifetime it is documented to have. runStdioServer returns void: no caller read the handle. Refs certinia#103
v4 removes key folding and path expansion. Every encode() call here passes no options, so no response changed and no golden moved. The other v4 encode forms were measured and none pay: keyed tabular costs a character per row, a tab or pipe delimiter saves 2 tokens on a 9,365-token response, nested field groups have nothing left to fold, and indentSize 1 saves 2.4% but decode rejects it. Recorded in DEVELOPING.md. Closes certinia#121
A group holds parents and their children alike, so summing durationTotalNs over every member counted the same time once for the child and again for every ancestor above it — 1.6x the transaction under groupBy: "namespace" on a real log, 3.1x on the test fixture. Only the members with no ancestor in the same group now add their total, so the figure answers "what does the transaction take back if this group never runs". It is not additive across rows, and the groupBy description says so: an agent that sums it would read 4.9x the transaction. listOperations threads the enclosing operation through walkLog, which now carries an inherited value down the tree. The parent link is internal and never reaches a response. The groupBy description costs 26 more definition tokens, so the budget is raised from 338 to 343 and both README token tables are regenerated. Closes certinia#101
Ranking single calls hid volume: a flow element that runs 373 times for 15% of a real transaction was absent from the top ten. The ten rows returned now cover 83% of the self time where they covered 51%. Group on kind + namespace + name, so one name in two namespaces stays two rows. Add durationSelfMaxMs, which read against durationSelfMs tells one bad call from sheer volume, and point lineNumber at that call. groupBy: "none" restores the per-call ranking. The response budgets rise to 325 and the definition budget to 372. Closes certinia#126
Memoize the group key: the nesting test walks the ancestors of every member, and a deep Apex stack rebuilt the same string at every level. Export GROUP_BY from the module that folds, so the tool schema cannot drift from it, and trim the groupBy description to what the agent acts on. The definition budget falls to 354.
lcottercertinia
approved these changes
Aug 14, 2026
lcottercertinia
pushed a commit
that referenced
this pull request
Aug 14, 2026
) Closes #127. Stacked on #133 (#126), which is stacked on #132 (#101). Review the top commit only — the rest is those two, and this diff shrinks as they merge. ## Why `namespace` on a row is the namespace of the *frame*, which is not always the namespace that asked for the work. `DMLBeginLine` pins `namespace = "default"` however the DML was reached (`ApexLogParser.ts:1412`), and because the tree builder inherits with `||=` the enclosing frame can never overwrite it. `SOQLExecuteBeginLine` and `SOSLExecuteBeginLine` carry no namespace logic at all, so they silently inherit the caller. One column, two meanings. Measured on a real log, `kind: "dml"`, 944 ms in total: | groupBy | result | | --- | --- | | `name` | four rows, every one `namespace: default` | | `callerNamespace` | 934.0 ms to the calling package, 10.3 ms to `default` | ## Why it is not a column Over two real logs the caller and the frame agree on almost every row: | log | rows | caller differs | | --- | ---: | ---: | | 19 MB, managed packages throughout | 2,469 | 72 (**2.9%**) | | 39 MB, every category at `INFO` | 39,415 | 2 (**0.0%**) | Flows, SOQL, workflow and methods differ on **0%** of rows. The *time* sits entirely in the rows that do differ — DML above, and `codeUnit` on 17% of rows carrying 10,600 of its 10,904 ms. So the fact matters and the column does not: it would repeat `namespace` on 97% of every response. `callerNamespace` is therefore carried on the internal `Operation` and never emitted. It is a `groupBy` value instead. The rule is written up in `DEVELOPING.md` beside the other shaping conventions. ## What changed - `Operation.callerNamespace`, taken from the enclosing operation via the `parent` link #101 added. No traversal change. - `GROUP_BY` gains `"callerNamespace"`. - `groupOperations` now takes the pair a folded row reports from one `IDENTITY_BY_GROUP` table rather than branching on `by` in two places. Same partition as before for `name` and `namespace`. ## Cost Response goldens **unmoved** — the enum value alone changes no response. The definition grows 337 → 361 tokens, the budget to 379; the extra is the clause that says what `callerNamespace` attributes, without which an agent cannot act on the value. `pnpm run build`, `pnpm test` (265), `pnpm run lint` and `pnpm run eval` all pass.
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.
Closes #126.
Stacked on #132 (#101). Review the top two commits only — the rest is #132, and this diff shrinks once that merges.
Why
Ranking single calls hid volume. On a real customer log,
FlowDecision myDecisionruns 373 times for 4,171.8 ms — 15% of the transaction — and appeared nowhere in the top ten. WithFlowAssignment myVariable_waitStartTimeAssignment(337 calls, 10%) that is a quarter of the run, invisible.The ten rows returned now cover 83% of the self time where they covered 51%.
What changed
groupBydefaults to"name", and gains"none"to restore the old per-call ranking.Groups key on
kind + namespace + name, so one name in two namespaces stays two rows rather than merging under whichever namespace was seen first. This matches Lana Analysis, which keys ontype|namespace|text.New
durationSelfMaxMson a grouped row: the self time of its slowest call. Read againstdurationSelfMsit separates one bad call from sheer volume, which need opposite fixes:DML Op:Update Type:c2g__codaInvoiceLineItem__cFlowDecision myDecisionThe mean is derivable from
callCount, so the mean is not sent.durationSelfMaxMsis omitted whengroupBy: "none", where it would repeatdurationSelfMs.lineNumberon a grouped row names that slowest call, in place of the oldnull.Budgets
Response budgets rise to 325, the definition budget to 354. Both raised deliberately, with the reason in the comment beside them. Goldens re-recorded and the diff read:
srm_pkgfolds tocallCount: 2,core_pkgto 3,List<Account>.add(Object)to 29, and the freed slots pull in three operations that were below the cap.The second commit is
/simplifyfollow-up: memoize the group key (the ancestor walk rebuilt the same string at every stack level), exportGROUP_BYso the tool schema cannot drift from the module that folds, and trim thegroupBydescription.Verification
pnpm run build,pnpm test(263),pnpm run lintandpnpm run evalall pass. Verified end to end against a real customer log outside this repo.