Repository navigation
Conversation
Contributor
Author
|
|
`bun run build:sdk` fails on main for every recent commit, and the binary build
stops behind it. Two independent causes:
1. common's JSON types were mutable - `{ [key: string]: JSONValue }` and
`JSONValue[]` - while the model SDKs type their payloads as `Readonly<...>`.
A mutable index signature does not accept a read-only one, so every
providerOptions assignment was a type error: seven of them, from
sdk/src/impl/llm.ts:269 through common/src/util/messages.ts and
agent-runtime's stream parsers. JSON that crosses a boundary is read-only,
so the type now says so. The one place that assembles a new payload
(agent-runtime's gravity-index `omitUndefined`) builds a writable
`Record<string, JSONValue | undefined>` and returns that.
2. cli/src imports `picocolors` in three files and it was declared in no
package.json, so the binary build stopped at `Could not resolve:
"picocolors"`. Declared at ^1.1.1.
Verified with the four steps CI runs, in that order, from a clean tree:
bun install --frozen-lockfile exit 0
bun run build:sdk exit 0 (was 7 TS errors and
"declaration bundling failed")
bun freebuff/cli/build.ts 0.0.0-ci exit 0
cli/bin/freebuff --version + smoke-binary.ts OK, boot matched
The compiled binary bakes NEXT_PUBLIC_* in at build time. The workflow set them
only on the smoke step, so the build ran with none of them: the values were
baked in as undefined, every invocation died in common/src/env.ts, and the smoke
step reported "--smoke-api-url exited 1 without printing a URL".
Reproduced from a clean environment (env -i, both directions):
build without NEXT_PUBLIC_* → --smoke-api-url exits 1 in the env schema
build with this env block → probe prints the URL; tree-sitter, dotenv
isolation and boot all pass
Salhi-zh
force-pushed
the
cli/root-text-coalescing
branch
from
October 29, 2026 20:07
e46ce57 to
c27324e
Compare
Contributor
Author
|
This branch currently sits on top of #1575 ( |
Contributor
Author
Memory, measuredSame updater, 28,593 streamed deltas (200 KB answer), heap sampled every 1,000 appends,
The retained data is identical (one text block either way); what disappears is the per-token copy of the block array — the allocation churn that the GC then has to collect between flushes, which is where the RSS growth and late-answer stutter come from. Scope: transient heap during streaming, not end-to-end process RSS of a full session. |
Green on this repo meant the tree built and the binary booted: no workflow step ran `bun test`, so a red suite and a green check looked identical. This adds typecheck and test steps for both packages, which only works once the suites can pass in the public export - four breakages: - `test-utils.ts` threw when `packages/internal/src/env` was absent, which is every public checkout (the directory is on the mirror's forbidden list). It now falls back to the defaults it already installs, and 22 suites load. - the catalog test read `fetch.mock.calls[0]` as the session POST, but device key registration is now the first call; it selects the session calls by URL. - the dock-arm test assumed no auth token, which is only true on a machine with no `~/.config/manicode/credentials.json`; it pins the token absent. - the Desktop mirror check read a file this export withholds; it skips when that file is absent and is unchanged in the private tree. - the runtime probe test asserted an empty list, which also carried the host's node/npx; it asserts no python, which is what it is about. `tar` (required by the release launcher) and `@types/react-dom` join the dev dependencies, which is what the wrapper and typecheck steps needed. cli: 4288 pass / 0 fail, sdk: 1070 pass / 0 fail, both typechecks 0 errors.
cli/bunfig.toml preloads ../test/setup-scm-loader.ts, but that file was never in the tree: `bun test` in the cli package died at startup with 'preload not found', so the suite could not run from a clean checkout at all. The other two preloads it names are tracked; this one was the gap. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
@opentui/react ships a prebuilt chunk that calls jsxDEV, and on a production NODE_ENV the React it resolves there does not export it: on the GitHub runner that alone failed 87 component tests with 'jsxDEV is not a function' inside opentui's error-boundary chunk, while the same suite passed locally. The suite tests our components, not React's production build, and NODE_ENV=test resolves the development runtime the chunk expects. cli: 4288 pass / 0 fail / 32 skip locally under the new value. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
The batched message updater applies its queue once per 100 ms window, but every streamed text delta goes into that queue as its own closure, so a flush calls appendTextToRootStream - and the shallow [...blocks] copy at its top - once per token. On a long answer that copy is the hot path: it is what turns a tag-heavy 2 MB stream into a 24 s stall. Queue text deltas as structured entries and merge each into the entry already at the tail of the queue when the text type matches, so a window costs one append per text type. appendRootChunk now calls appendRootText instead of closing over appendTextToRootStream. Measured (200 KB answer, 26,572 deltas): 618.3 ms of block work becomes 12.7 ms at 100 deltas per flush (48.8x; 22.4 ms at 50, 35.4 ms at 25). On the 2,040,000-char tag-heavy stream (291,429 deltas, 20,001 blocks): 23,949.6 ms flushing every delta, 262.3 ms at 200 per flush - 91x. Deltas merge only when adjacent in the queue, so ordering against tool blocks and other updates is unchanged; two tests pin that, and one pins that a merged flush produces the same block shape as feeding the primitive one delta at a time. A delta now reaches the screen up to one flush window later, which is the window the updater already had.
Salhi-zh
force-pushed
the
cli/root-text-coalescing
branch
from
October 29, 2026 21:22
c27324e to
4792450
Compare
This branch has not been deployed
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
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.
Why
The batched message updater applies its queue once per 100 ms, but that batching was only half done: every streamed
text/reasoningdelta was queued as its own closure, so a flush calledappendTextToRootStreamonce per token. That function starts withconst nextBlocks = [...blocks]— a shallow copy of every block in the message — so a long answer copied the whole block array per token.What changes
PendingUpdatebecomes a tagged union:{ kind: 'updater' }for existing queued closures,{ kind: 'rootText', delta }for streamed text.appendRootTextmerges a delta into the tail entry when both arerootTextof the same type, so consecutive deltas collapse into one string per window. An interleaved non-text update ends the merge, so ordering is preserved by construction.flushcomposes the queue into a single transform and applies it in onesetMessages, appending text once per text type per window.appendRootChunk(utils/sdk-event-handlers.ts) callsupdater.appendRootText(delta)instead of wrappingappendTextToRootStreamin a closure.No primitive, component or render path changes.
Performance
bun1.4.2, same machine:The second row is the case that produced a multi-second stall late in a long, tag-heavy reply: the cost was the per-token block-array copy, and at the shipped cadence the product no longer pays it.
Verification
mainis red for an unrelated reason (sdk/src/impl/llm.tsprovider options, 23 errors incli), so the typecheck line is a diff of the error output before and after this branch, not an exit-0 claim.New tests:
merged deltas produce the same blocks as one append each— the same delta sequence through the batched updater and, one at a time, throughappendTextToRootStream; the block shapes (content,textType,thinkingOpen) must be equal.a flush boundary and an interleaved update keep arrival order— text, flush, more text, an interleaved block update, flush: the text must readbefore after|tool output.Risk and rollback
thinktag the parser already mis-splits, and merging removes split points rather than adding them. On an adversarial 7-char-split fixture the coalesced path preserved 5,405–5,546 of 5,640 text characters where per-delta feeding preserved 47. Fixing the primitive is a separate change (cli: scan only the seam for a completable think tag #1573 is the same area).Base
Sits on #1575 (the build fix) and #1577 (CI now runs the suites). Rebased onto that pair, so this PR's checks run
bun testandtscfor real instead of only building the tree and smoke-testing the binary.