Skip to content

Commit 672b89d

Browse files
fix(embedding): split oversized canonical lines so chunk windows never exceed the provider context (#206) (#207)
* fix(embedding): split oversized canonical lines so chunk windows never exceed the provider context (#206) chunkCanonicalText windowed compartment chunk-embedding input only at canonical line (U:/A: span) boundaries. A single line larger than the per-window token budget was emitted whole, so a compartment span containing a large message (e.g. a file dump rendered into one A: span) produced one window far over the provider's hard context window — jina via litellm returned 400 exceed_context_size for a 51774-token window against an 8192 ceiling, and the compartment could never be embedded. Split a single oversized line down to the per-window budget before emitting its windows, using @langchain/textsplitters RecursiveCharacterTextSplitter with a lengthFunction backed by the existing estimateTokens tokenizer, so slicing is token-accurate and deterministic (cache-safe, no provider call). Each sub-slice becomes its own window carrying the owning line's ordinal range; a char-budget fallback guarantees termination on token-dense text with no separators. windowIndex stays 1-based contiguous so stored chunk identity (compartmentId + windowIndex + hash) is preserved. chunkCanonicalText is now async (the splitter API is async); the two production callers and the test seams await it. Adds tests: a single oversized line splits into multiple in-budget windows with stable ordinals/indices, and split sub-windows interleave with normal line windows without index gaps. * fix(embedding): address PR #207 review — vendor splitter, bound provider calls, log fallback Review feedback (greptile/cubic/socket) on #207: - Drop the @langchain/textsplitters dependency (tripped an org Socket "obfuscated code" alert on its minified dist) and vendor a minimal synchronous port of RecursiveCharacterTextSplitter as recursive-text-splitter.ts (algorithm + separator hierarchy ported from v1.0.1, MIT). This also reverts chunkCanonicalText back to sync, removing the async ripple through callers and tests. - Bound provider call size when a SINGLE compartment produces more windows than MAX_WINDOWS_PER_EMBED_CALL: embedTextsWindowBounded sub-batches the texts across provider calls and concatenates vectors, so the slice builder's "always include at least one compartment whole" rule can no longer hand the provider one enormous payload (cubic P2). Per-compartment persistence/retry accounting is unchanged. - Log when the recursive splitter throws before falling back to the char-budget split, instead of swallowing the error silently (greptile/cubic P2). - Clarify the charBudgetSplit budget guarantee: every slice is within budget except the degenerate single-character case under a tiny budget, which is unreachable with real provider budgets (cubic P3). Tests: vendored splitter unit tests (separator fallthrough, char split, custom length fn, round-trip); a #207 batching test asserting no provider call exceeds the window cap when one compartment yields many windows. 55 tests pass across the affected files; tsc + biome clean. * fix(embedding): re-check char-budget split output so no over-budget slice escapes (PR #207) greptile flagged that splitOversizedLine's final guard pushed charBudgetSplit sub-slices to the result unchecked, so a degenerate single-character slice that alone exceeds the budget could escape the 'no window over budget' guarantee. Route every charBudgetSplit sub-slice through a checked push that re-splits any still-over-budget slice with length > 1 and only emits a lone irreducible character as the terminal case. * test(embedding): drop stale async on test callbacks after chunkCanonicalText went sync chunkCanonicalText reverted to synchronous when the splitter was vendored, but several test callbacks kept their now-pointless async marker (no remaining await). Strip async from the 7 affected test blocks. Production embedTextsWindowBounded stays async — it awaits real provider calls. * fix(embedding): coerce empty input to a space at the provider chokepoint Some OpenAI-compatible providers (jina via litellm) reject an empty-string embedding input with HTTP 400 "Input content cannot be empty", which fails the WHOLE batch (response is all-null) — so one empty input blocks embedding every other text sent with it, and the affected compartment/memory loops as "could not be embedded" forever. Coerce empty/whitespace-only inputs to a single space (verified to embed) in OpenAICompatibleEmbeddingProvider.embedBatch, before the POST. This is the single chokepoint covering every caller (chunk, memory, query), independent of any provider-side hook. Real text and token-id-shaped inputs are untouched; result mapping is unchanged (coercion preserves array length 1:1).
1 parent 2ccf3de commit 672b89d

9 files changed

Lines changed: 538 additions & 8 deletions

bun.lock

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/plugin/src/features/magic-context/compartment-chunk-embedding.test.ts

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,13 @@ describe("compartment chunk embedding core", () => {
138138
expect(whole[0]).toMatchObject({ windowIndex: 0, startOrdinal: 1, endOrdinal: 3 });
139139
expect(whole[0]?.text).toBe(text);
140140

141-
const windowed = chunkCanonicalText(text, 1, 3, 1);
141+
// Budget that fits any single line but not two together → one window per
142+
// line on line boundaries. effectiveMax = floor(budget * 0.9); each line is
143+
// ~7 tokens, so a budget of ~9 (effective 8) holds exactly one line.
144+
const perLineBudget = Math.ceil(
145+
(estimateTokens("[1] U: alpha beta gamma") + 1) / CHUNK_WINDOW_SAFETY_RATIO,
146+
);
147+
const windowed = chunkCanonicalText(text, 1, 3, perLineBudget);
142148
expect(windowed.map((window) => window.windowIndex)).toEqual([1, 2, 3]);
143149
expect(windowed.map((window) => [window.startOrdinal, window.endOrdinal])).toEqual([
144150
[1, 1],
@@ -167,6 +173,50 @@ describe("compartment chunk embedding core", () => {
167173
}
168174
});
169175

176+
test("splits a single oversized canonical line so no window exceeds the budget (#206)", () => {
177+
// One canonical line (a single A: span) far larger than the budget — e.g.
178+
// a big file dump rendered into one message. The old chunker emitted this
179+
// whole, producing one window that blew past the provider's context window.
180+
const maxInputTokens = 200;
181+
const effective = Math.floor(maxInputTokens * CHUNK_WINDOW_SAFETY_RATIO);
182+
const huge = Array.from(
183+
{ length: 4000 },
184+
(_, i) => `word${i} alpha beta gamma delta epsilon`,
185+
).join(" ");
186+
const line = `[1] A: ${huge}`;
187+
expect(estimateTokens(line)).toBeGreaterThan(effective * 10); // genuinely oversized
188+
189+
const windows = chunkCanonicalText(line, 1, 1, maxInputTokens);
190+
191+
expect(windows.length).toBeGreaterThan(1);
192+
// The invariant that #206 violated: NO window may exceed the budget.
193+
for (const window of windows) {
194+
expect(estimateTokens(window.text)).toBeLessThanOrEqual(effective);
195+
}
196+
// Sub-windows all carry the owning line's ordinal range.
197+
for (const window of windows) {
198+
expect(window.startOrdinal).toBe(1);
199+
expect(window.endOrdinal).toBe(1);
200+
}
201+
// windowIndex stays 1-based and contiguous (stable chunk identity).
202+
expect(windows.map((w) => w.windowIndex)).toEqual(windows.map((_, i) => i + 1));
203+
});
204+
205+
test("mixes split sub-windows with normal line windows without index gaps", () => {
206+
const maxInputTokens = 200;
207+
const effective = Math.floor(maxInputTokens * CHUNK_WINDOW_SAFETY_RATIO);
208+
const huge = Array.from({ length: 2000 }, (_, i) => `tok${i}`).join(" ");
209+
const text = ["[1] U: short opener", `[2] A: ${huge}`, "[3] U: short closer"].join("\n");
210+
211+
const windows = chunkCanonicalText(text, 1, 3, maxInputTokens);
212+
213+
expect(windows.length).toBeGreaterThan(2);
214+
for (const window of windows) {
215+
expect(estimateTokens(window.text)).toBeLessThanOrEqual(effective);
216+
}
217+
expect(windows.map((w) => w.windowIndex)).toEqual(windows.map((_, i) => i + 1));
218+
});
219+
170220
test("storage replaces chunks idempotently and clearSession removes rows", () => {
171221
const db = createDb();
172222
try {

packages/plugin/src/features/magic-context/compartment-chunk-embedding.ts

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { createHash } from "node:crypto";
2+
23
import { estimateTokens } from "../../hooks/magic-context/read-session-formatting";
34
import { getHarness } from "../../shared/harness";
5+
import { log } from "../../shared/logger";
46
import type { Database, Statement as PreparedStatement } from "../../shared/sqlite";
7+
import { recursiveCharacterSplit } from "./recursive-text-splitter";
58

69
export const DEFAULT_COMPARTMENT_CHUNK_MAX_INPUT_TOKENS = 512;
710

@@ -501,6 +504,28 @@ export function chunkCanonicalText(
501504
const lineStart = range?.start ?? startOrdinal;
502505
const lineEnd = range?.end ?? lineStart;
503506
const lineTokens = estimateTokens(line);
507+
508+
// A single canonical line (one U:/A: span) can itself exceed the per-window
509+
// budget — e.g. a span containing a large file dump or paste rendered into
510+
// one line. Packing only flushes BETWEEN lines, so such a line would be
511+
// emitted as one oversized window and blow past the provider's hard context
512+
// window (#206: jina returned 400 exceed_context_size for a 51774-token
513+
// window against an 8192 ceiling). Split the line down to budget first, and
514+
// emit each sub-slice as its own window carrying this line's ordinal range.
515+
if (lineTokens > effectiveMax) {
516+
flush();
517+
for (const slice of splitOversizedLine(line, effectiveMax)) {
518+
windows.push({
519+
windowIndex: windows.length + 1,
520+
startOrdinal: lineStart,
521+
endOrdinal: lineEnd,
522+
text: slice,
523+
chunkHash: hashChunkText(slice),
524+
});
525+
}
526+
continue;
527+
}
528+
504529
if (currentLines.length > 0 && currentTokens + lineTokens > effectiveMax) {
505530
flush();
506531
}
@@ -513,9 +538,108 @@ export function chunkCanonicalText(
513538
}
514539
flush();
515540

541+
// windowIndex is assigned contiguously as 1-based at push time (both the
542+
// flush path and the oversized-line split use `windows.length + 1`), so it is
543+
// already gap-free and stable — preserve it (chunk identity = compartmentId +
544+
// windowIndex + hash; renumbering would orphan every stored chunk row).
516545
return windows;
517546
}
518547

548+
/**
549+
* Split a single oversized canonical line into sub-slices each within
550+
* `effectiveMax` tokens, using a recursive character splitter (best-in-class
551+
* boundary hierarchy: paragraph → line → sentence → word → char). The
552+
* `lengthFunction` is our real tokenizer (`estimateTokens`), so slicing is
553+
* token-accurate against the same heuristic the windower uses — deterministic,
554+
* no provider call, cache-stable. A hard char-level safety cap guarantees
555+
* termination even if a single token-dense fragment resists separator splitting.
556+
*/
557+
function splitOversizedLine(line: string, effectiveMax: number): string[] {
558+
// Recursive split on the best-in-class separator hierarchy (paragraph → line
559+
// → word → char), measured with our real tokenizer. Fall back to a
560+
// deterministic char-budget split if the splitter throws or yields nothing
561+
// (never leave an oversized line un-split).
562+
let slices: string[] = [];
563+
try {
564+
slices = recursiveCharacterSplit(line, {
565+
chunkSize: effectiveMax,
566+
lengthFunction: estimateTokens,
567+
});
568+
} catch (error) {
569+
// Surface the regression instead of degrading silently: if the splitter
570+
// consistently fails for some input shape the char-budget fallback still
571+
// embeds, but we want a signal.
572+
log("[magic-context] recursiveCharacterSplit failed; using char-budget fallback:", error);
573+
slices = [];
574+
}
575+
if (slices.length === 0) {
576+
slices = charBudgetSplit(line, effectiveMax);
577+
}
578+
// Final guard: any slice still over budget (token-dense, no separators) is
579+
// hard-split by character budget. charBudgetSplit is the TERMINAL splitter —
580+
// it shrinks to a single character, the smallest indivisible unit — so its
581+
// output is budget-compliant by construction EXCEPT for the degenerate case
582+
// of a lone character that alone exceeds the budget (only reachable with a
583+
// tiny effectiveMax; never with real provider budgets). We assert that
584+
// contract in dev/test rather than re-splitting (which cannot reduce a
585+
// 1-char slice further and would loop): any escapee is a genuine bug, not
586+
// something to silently paper over.
587+
const safe: string[] = [];
588+
const pushChecked = (slice: string): void => {
589+
if (estimateTokens(slice) > effectiveMax && slice.length > 1) {
590+
// Not terminal yet — split further. (Defensive: charBudgetSplit
591+
// should already guarantee this; only triggers if its contract
592+
// regresses.)
593+
safe.push(...charBudgetSplit(slice, effectiveMax));
594+
return;
595+
}
596+
safe.push(slice);
597+
};
598+
for (const slice of slices) {
599+
if (estimateTokens(slice) <= effectiveMax) {
600+
safe.push(slice);
601+
} else {
602+
for (const sub of charBudgetSplit(slice, effectiveMax)) pushChecked(sub);
603+
}
604+
}
605+
return safe.filter((s) => s.length > 0);
606+
}
607+
608+
/**
609+
* Deterministic character-budget fallback split. Estimates a chars-per-token
610+
* ratio from the input and slices on that, then trims each slice down until it
611+
* fits the token budget. Always terminates.
612+
*
613+
* Budget guarantee: every emitted slice is within `effectiveMax` tokens EXCEPT
614+
* the degenerate case where a single character already exceeds the budget (only
615+
* reachable when `effectiveMax` is tiny — e.g. 1 — and one char tokenizes to
616+
* multiple tokens). In that case the slice is a single character: it cannot be
617+
* split further, so emitting it is the only progress-making choice. With real
618+
* provider budgets (thousands of tokens) this case never arises.
619+
*/
620+
function charBudgetSplit(text: string, effectiveMax: number): string[] {
621+
const totalTokens = Math.max(1, estimateTokens(text));
622+
const charsPerToken = Math.max(1, Math.floor(text.length / totalTokens));
623+
const sliceChars = Math.max(1, effectiveMax * charsPerToken);
624+
const out: string[] = [];
625+
let pos = 0;
626+
while (pos < text.length) {
627+
let end = Math.min(text.length, pos + sliceChars);
628+
let slice = text.slice(pos, end);
629+
// Shrink until the slice fits the token budget (handles dense regions).
630+
// Floor at one character: a single char is the smallest indivisible unit,
631+
// so we stop there even if it alone exceeds the budget (degenerate tiny
632+
// budget) — otherwise the loop could not make progress.
633+
while (slice.length > 1 && estimateTokens(slice) > effectiveMax) {
634+
end = pos + Math.max(1, Math.floor((end - pos) / 2));
635+
slice = text.slice(pos, end);
636+
}
637+
out.push(slice);
638+
pos = end;
639+
}
640+
return out;
641+
}
642+
519643
export function getExistingChunkHashes(
520644
db: Database,
521645
compartmentId: number,

packages/plugin/src/features/magic-context/memory/embedding-openai.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,19 @@ describe("OpenAICompatibleEmbeddingProvider request body (NVIDIA NIM fields, iss
153153
expect(body.input).toBeDefined();
154154
});
155155

156+
test("coerces empty / whitespace-only input to a space so the provider can't 400 the batch", async () => {
157+
const provider = new OpenAICompatibleEmbeddingProvider({
158+
endpoint: "http://127.0.0.1:65535",
159+
model: "text-embedding-3-small",
160+
});
161+
fetchSpy.mockImplementation((async () => successResponse()) as FetchLike);
162+
await provider.embedBatch(["", " ", "real text", "\n\t"]);
163+
const init = fetchSpy.mock.calls[0]?.[1] as RequestInit;
164+
const body = JSON.parse(init.body as string) as { input: string[] };
165+
// Empty / whitespace inputs become a single space; real text is untouched.
166+
expect(body.input).toEqual([" ", " ", "real text", " "]);
167+
});
168+
156169
test("purpose query sends queryInputType when configured (#155)", async () => {
157170
const provider = new OpenAICompatibleEmbeddingProvider({
158171
endpoint: "http://127.0.0.1:65535",

packages/plugin/src/features/magic-context/memory/embedding-openai.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,16 @@ export class OpenAICompatibleEmbeddingProvider implements EmbeddingProvider {
198198
return [];
199199
}
200200

201+
// Coerce empty / whitespace-only inputs to a single space before the POST.
202+
// Some OpenAI-compatible providers (e.g. jina via litellm) reject an empty
203+
// string with HTTP 400 "Input content cannot be empty", which fails the
204+
// WHOLE batch (the response is all-null) — so one empty input would block
205+
// embedding every other text sent with it. A space embeds fine (verified)
206+
// and yields a stable near-zero-information vector. Callers should avoid
207+
// sending empty content where possible, but this is the single chokepoint
208+
// that guarantees a stray empty string can't 400 the request.
209+
const requestTexts = texts.map((t) => (t.trim().length === 0 ? " " : t));
210+
201211
if (!(await this.initialize())) {
202212
return Array.from({ length: texts.length }, () => null);
203213
}
@@ -245,7 +255,7 @@ export class OpenAICompatibleEmbeddingProvider implements EmbeddingProvider {
245255
},
246256
body: JSON.stringify({
247257
model: this.model,
248-
input: texts,
258+
input: requestTexts,
249259
// Optional provider-specific fields (e.g. NVIDIA NIM requires
250260
// input_type; truncate is accepted by several providers).
251261
// Omitted entirely when unset so standard OpenAI endpoints are

packages/plugin/src/features/magic-context/project-embedding-registry.test.ts

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,36 @@ function seedManyCompartmentsWithFts(
167167
}
168168
}
169169

170+
/**
171+
* Seed one compartment whose single assistant message is huge, so its canonical
172+
* chunk text is one oversized line that chunkCanonicalText splits into many
173+
* windows — the #207 batching case.
174+
*/
175+
function seedOversizedCompartmentWithFts(
176+
db: NonNullable<ReturnType<typeof openDatabase>>,
177+
sessionId: string,
178+
): number {
179+
appendCompartments(db, sessionId, [
180+
{
181+
sequence: 0,
182+
startMessage: 1,
183+
endMessage: 1,
184+
startMessageId: "a1",
185+
endMessageId: "a1",
186+
title: "Giant dump",
187+
content: "P1 content",
188+
p1: "P1 content",
189+
},
190+
]);
191+
// ~6000 words ≈ thousands of tokens » the default chunk budget, all in one
192+
// assistant message → one oversized canonical line.
193+
const huge = Array.from({ length: 6000 }, (_, i) => `word${i}`).join(" ");
194+
db.prepare(
195+
"INSERT INTO message_history_fts (session_id, message_ordinal, message_id, role, content) VALUES (?, ?, ?, ?, ?)",
196+
).run(sessionId, 1, `${sessionId}-a1`, "assistant", huge);
197+
return getCompartments(db, sessionId)[0].id;
198+
}
199+
170200
describe("project embedding registry", () => {
171201
const tempDirs: string[] = [];
172202
const originalXdgDataHome = process.env.XDG_DATA_HOME;
@@ -641,7 +671,7 @@ describe("project embedding registry", () => {
641671
expect(loadAllEmbeddings(db, projectIdentity, second.modelId).size).toBe(1);
642672
});
643673

644-
it("keeps old-model compartment chunk embeddings inert on provider change", () => {
674+
it("keeps old-model compartment chunk embeddings inert on provider change", async () => {
645675
const db = useTempDb();
646676
const compartmentId = seedCompartmentWithFts(db, "ses-wipe");
647677
const windows = chunkCanonicalText("[1] U: hello", 1, 1, 10_000);
@@ -716,6 +746,51 @@ describe("project embedding registry", () => {
716746
expect(batchCalls).toBe(1);
717747
});
718748

749+
it("bounds provider call size even when one compartment has many windows (#207)", async () => {
750+
const callSizes: number[] = [];
751+
_setTestProviderFactoryForProject(
752+
(config) =>
753+
new (class extends FakeEmbeddingProvider {
754+
override async embedBatch(texts: string[]): Promise<Float32Array[]> {
755+
callSizes.push(texts.length);
756+
return super.embedBatch(texts);
757+
}
758+
})(config.provider === "local" ? config.model : "off"),
759+
);
760+
const db = useTempDb();
761+
const compartmentId = seedOversizedCompartmentWithFts(db, "ses-huge");
762+
recordSessionProjectIdentity(db, "ses-huge", "git:huge");
763+
registerProjectEmbedding(
764+
db,
765+
"git:huge",
766+
localConfig("model-a"),
767+
{ memoryEnabled: true, gitCommitEnabled: false },
768+
"/tmp/huge",
769+
);
770+
771+
const embedded = await embedUnembeddedCompartmentChunksForProject(db, "git:huge");
772+
expect(embedded).toBe(1);
773+
774+
// The single compartment produced many windows; assert NO provider call
775+
// exceeded the per-call window cap (MAX_WINDOWS_PER_EMBED_CALL = 2), i.e.
776+
// the windows were sub-batched across calls rather than sent as one
777+
// enormous payload.
778+
expect(callSizes.length).toBeGreaterThan(1);
779+
for (const size of callSizes) {
780+
expect(size).toBeLessThanOrEqual(2);
781+
}
782+
783+
// And the compartment is fully embedded (one row per window, all persisted).
784+
const rows = loadCompartmentChunkEmbeddingsForSearch(
785+
db,
786+
"ses-huge",
787+
"git:huge",
788+
currentChunkModelId("git:huge"),
789+
);
790+
expect(rows.length).toBeGreaterThan(1);
791+
expect(new Set(rows.map((r) => r.compartmentId))).toEqual(new Set([compartmentId]));
792+
});
793+
719794
it("keeps passive chunk backfill scoped to the caller project", async () => {
720795
_setTestProviderFactoryForProject(
721796
(config) =>
@@ -763,7 +838,7 @@ describe("project embedding registry", () => {
763838
).toHaveLength(0);
764839
});
765840

766-
it("repairs chunk rows stamped with a different project than their session owner", () => {
841+
it("repairs chunk rows stamped with a different project than their session owner", async () => {
767842
const db = useTempDb();
768843
const compartmentId = seedCompartmentWithFts(db, "ses-repair");
769844
const windows = chunkCanonicalText("[1] U: hello", 1, 1, 10_000);

0 commit comments

Comments
 (0)