Unwedge the code source: a URL in a template literal, an 8192-token chunk, and an unbounded retry - #161
Merged
Conversation
Three red tests, one per link in the chain that kept mcp.copilotkit.ai's `code` source stuck at commit 0d0ea901 for ten days: - the code chunker latches `inTemplateString` on a `//` inside a template literal and then emits the whole rest of the file as one chunk; - the OpenAI provider's 30,000-CHARACTER cap does not bound TOKENS, so an oversized chunk hard-400s instead of being shrunk and retried; - the orchestrator holds the state token for a failing item forever, so one permanently-failing file freezes the entire source.
The code chunker's block-state tracker stripped ' and " strings before
counting backticks, but knew nothing about backticks themselves. So in
wsUrl: `ws://127.0.0.1:5177/inspector-lab-runtime/${key}/realtime`,
the `//` read as a line comment, the rest of the line — closing backtick
included — was discarded, the backtick count came out odd, and the tracker
latched inTemplateString for the remainder of the file. With the latch stuck
no blank line qualified as a split point again, so the last ~960 lines of
CopilotKit's threads-state-lab.ts became a single 36 KB chunk that OpenAI
rejected at 8192 tokens.
Replaces the two-stage strip-then-count with one left-to-right scan where the
quote, template and comment contexts are mutually exclusive — the only way
`//` can be classified correctly — and adds a hard 12,000-character bound on
any emitted chunk, because the blank-line and mechanical fallbacks bound LINES
and nothing bounded characters.
Measured over the 1,120 files the production 'code' source indexes: chunks
over 12,000 chars went 15 -> 0, and threads-state-lab.ts went from 3 chunks
(largest 27,785 chars) to 8 (largest 11,753).
OpenAIEmbeddingProvider capped inputs at 30,000 CHARACTERS to stay under an 8192-TOKEN limit. That holds only at 3.66 chars/token; source code tokenizes denser, so a 30,000-character code chunk is comfortably past 10,000 tokens and comes back as a non-retryable 400. A length rejection is recoverable by sending less text, so it is now handled separately from the generic retry path: the provider parses the input[N] the API named, halves that input, and retries — bounded at 12 rounds and a 64-char floor so it always terminates. Every shrink logs a warning naming the truncation, and any other 400 still throws. No fixed character cap can be right for every tokenizer; converging on the real limit empirically is what makes an over-length input non-fatal.
The orchestrator refuses to advance a source's state token past an item that failed, so the failure is retried rather than silently skipped. With an unbounded retry that is a trap: an item failing for a permanent reason fails identically forever. mcp.copilotkit.ai's 'code' source sat at commit 0d0ea901 for ten days — serving ten-day-old source-code search results to every user — because one file produced a chunk larger than the embedding model's limit. index_state now carries an item_failures ledger: consecutive-failure count, first-failure time and last error, per item id. Three consecutive failing runs quarantines an item — it stops holding the state token, the rest of the source indexes, and every run logs the id, the attempt count and the error. One is too few: a network blip, a rate limit and a poison file all look identical on their first failure, and quarantining those would trade a visible wedge for invisible data loss. Quarantine is advisory, not a blocklist. A quarantined item is still handed to the pipeline on every run, and a success clears its record outright. /health and the admin index-stats op gained quarantined_items — id, attempts, since, error — following the next_acquire precedent from #160, so an item missing from the index is never an invisible gap.
check-test-shapes flags mockResolvedValueOnce in a file that clears rather than resets mocks: clearAllMocks drains the call log but not the once-queue, so an unconsumed value leaks into a later test's first call.
jpr5
added a commit
that referenced
this pull request
Sep 11, 2026
…to see (#162) ## The audit could not report an index that had shrunk `reindex-audit.ts` Check 3 only ever fired on `db_has_more`. The `db_has_fewer` direction was in the finding type and was never implemented: ```ts // Check 3 — Count divergence (db_has_more only; db_has_fewer is expected // when the indexer filters low-semantic-value files like SVGs, base64, etc.) if (dbCount > diskCount) { ... } ``` The stated reason is real — the indexer legitimately drops files with no extractable prose — but "some shortfall is normal" got implemented as "all shortfall is invisible." ## Motivation: three bugs the check watched and never mentioned 1. **Wrong crawl scope.** The CopilotKit API reference (184 `.mdx`) lived in a sibling directory outside the configured walk root; 183 of 184 pages were live on the site and absent from the index. 2. **A wedged item.** The `code` source sat in a false `error` from 2026-09-01 to 2026-09-11: one file produced a 36 KB chunk and a non-retryable 400, and the orchestrator refused to advance past it. 3. **Successful extraction of nothing.** 130 of 682 prose `.mdx` files are pure JSX stubs whose text lives in the excluded `src/content/snippets/**`. They are walked, matched, read, stripped to empty, and chunk to zero. `markdown.ts` returns `[]`, `pipeline.ts` writes nothing — both silently. A 19% shortfall that stood for months. Honest scope note: this change catches #2 and #3. It does **not** catch #1 — when the walk root itself is wrong, the disk side of the comparison is wrong too, so disk and index agree. #1 needs a config-level scope check, not this one. ## What changed **Check 3 reports both directions.** The shortfall is measured on the **set difference** (disk minus index), not on `diskCount - dbCount`. The raw delta is a lossy proxy: a source holding one stale row and missing one real file has matching counts and reports nothing — the same blindness in miniature. The set difference fires in every case the count comparison would, plus that one, and it yields the actual paths, so the finding **names the files** instead of saying "the count is off by 130." **Keeping the signal from becoming noise.** The finding fires only above a per-source baseline: `unindexed_tolerance`, a new optional file-source key, defaulting to **5% of walked files**, with an **absolute floor of 3 files** so a small source does not alert on one or two legitimately-empty ones. The floor is bypassed when a source explicitly sets `0`. **What a healthy source looks like after this change:** it emits nothing. A source dropping a handful of empty or content-free files per reindex stays silent at the default. A source that has been cleaned up sets `unindexed_tolerance: 0` and then hears about the very first regression — that is the intended end state, and it is why the lever is per-source rather than a global mute. The failure mode this design is guarding against is an alarm that fires on every source on every reindex and gets muted within a week, which is how the direction came to be suppressed outright in the first place. `unindexed_tolerance` is deliberately **not** in the source fingerprint (`source-fingerprint.ts` uses an explicit allowlist): it changes reporting, not what gets indexed, so changing it must not force a reindex. **Dedup key now includes the direction.** A source can produce both directions in one run; `source:check` alone let one overwrite the other and suppress its Slack alert. **The two silent drop sites are now audible.** Every `return []` in `chunkMarkdown` warns with the file and a *distinct* reason — empty file / MDX stripping left no prose / every split trimmed to empty — because those have completely different fixes. (There was a third silent path at the bottom of the function; it got the same treatment.) `pipeline.indexItem` logs that the index actually lost the item, prefixed by source and id, greppable next to `next_acquire_reason` and `quarantined_items`: ``` [chunker] no chunks for docs/quickstart.mdx: MDX stripping left no prose (the file is JSX/imports only — its text may live in a snippet or component that is not indexed); it will not be indexed [pipeline:docs] docs/quickstart.mdx produced zero chunks; nothing indexed (any previously indexed chunks for it are being cleared) ``` ## Should a zero-chunk file be its own audit finding? **No — and it does not need to be, because the shortfall finding already names it individually.** A zero-chunk file is written as `replaceChunksForFile(src, id, [])`, which deletes the row, so it is *by construction* a disk file absent from the index and it lands in the shortfall finding's `samples` by path. A separate persisted zero-chunk finding would need the indexer to write a second record of the same fact into a second store, which can go stale against the chunks table and would report a file as dropped after someone fixed it. The audit's two sources of truth are disk and the index; keeping it that way means the finding cannot lie. The per-file *reason* — which the audit genuinely cannot know — is what the new chunker warnings supply, at the one place that does know. ## Red-green **RED** — 40 `.mdx` pages, 8 of them pure-JSX stubs, run through the **real** chunker and the **real** pipeline (only the DB and the disk walk are faked), so the test exercises the actual drop path: ``` stdout | reports a db_has_fewer finding when pure-JSX stubs chunk to zero RED PROOF findings = [] × reports a db_has_fewer finding when pure-JSX stubs chunk to zero AssertionError: expected undefined to be defined ``` Preconditions in the same run passed: disk 40 files, index 32. The shrink was real and the audit said nothing. **GREEN** — same test, unchanged: ``` stderr | [reindex-audit] docs — count_divergence: 8 issues (db_has_fewer): page-0.mdx, page-5.mdx, page-10.mdx, page-15.mdx, page-20.mdx stdout | GREEN_PROOF [{"source":"docs","check":"count_divergence","count":8, "samples":["page-0.mdx","page-5.mdx","page-10.mdx","page-15.mdx","page-20.mdx", "page-25.mdx","page-30.mdx","page-35.mdx"],"direction":"db_has_fewer"}] ✓ reports a db_has_fewer finding when pure-JSX stubs chunk to zero ``` **Negative assertion** — a source skipping a normal share (1 of 40, 2.5%) produces `findings === []`, and the same shortfall *does* report once the source sets `unindexed_tolerance: 0`: ``` ✓ does NOT report a shortfall for a source skipping a normal share of files ✓ reports that same small shortfall once the source sets unindexed_tolerance: 0 ``` **Mutation-tested** so the negative assertion is not vacuous — replacing `unindexed.length > budget` with `> 0` fails it: ``` × does NOT report a shortfall for a source skipping a normal share of files Tests 1 failed | 2 passed (3) ``` The chunker and pipeline warnings were red-greened the same way (2 chunker tests and 1 pipeline test failed before the warnings existed), each paired with a "does not warn on a normal file" assertion. ## Local gate `npm run build` ✅ · `npx tsc --noEmit` ✅ · `npx tsc --noEmit -p tsconfig.scripts.json` ✅ · `node scripts/check-test-shapes.mjs` ✅ · `npx prettier --check` ✅ · `npm test` ✅ **189 passed | 1 skipped (190 files), 3712 passed | 1 skipped**. No pre-existing failures. ## What an operator does when this fires Open the named files. Three outcomes, in order of likelihood: 1. The files are stubs or otherwise have no indexable prose — real but legitimate. Either fix the extraction (inline the snippet, widen the source) or, if the drop is genuinely correct, raise that source's `unindexed_tolerance` past the measured baseline and leave a note saying why. 2. The files should have indexed and did not — grep the reindex logs for `[chunker] no chunks for <path>` and `produced zero chunks`; the reason is there. 3. No chunker warning for them at all — the source never got that far. Check `/health` for a wedged source and a held state token (#160, #161).
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
1 participant
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.
The failing item
packages/web-inspector/dev/threads-state-lab.ts, in the CopilotKit repo, indexed by thecodesource on mcp.copilotkit.ai. Production logs it on every run:input[2]— the third chunk — was 35,964 characters covering lines 159 through 1121 of a 1121-line file.Root cause, three links
1. The chunker latches on a URL inside a template literal.
stripStringsAndLineComments(src/indexing/chunking/code.ts) tracked'and"but not backticks. Line 262 of the file is:The
//inws://read as a line comment, so everything afterws:— including the closing backtick — was discarded. The backtick count came out odd,trackBlockStatesetinTemplateString = true, and nothing ever cleared it. With the latch stuck,findSplitPointsrejected every subsequent blank line, and the last ~960 lines of the file collapsed into one chunk. Instrumenting the shipped heuristic over the real file shows exactly one state transition in 1121 lines:L262 tmpl:false->true, and it never comes back.2. The embedding provider's cap is in the wrong unit.
MAX_CHARS = 30_000was commented as "~8192 tokens with safety margin". That holds only at 3.66+ chars/token; source code tokenizes much denser, so 30,000 characters of TypeScript is comfortably over 10,000 tokens. The 400 is not retryable, so the item failed hard.3. The orchestrator retries forever.
indexSourceWithStatedeliberately refuses to advance the state token past a failed item, so the failure is retried rather than skipped. Correct for a transient failure, fatal for a permanent one: the same item failed identically every run, and the source never moved off commit0d0ea901.Blast radius
Measured with the shipped chunker over the 1,120 files the production
codesource actually indexes (CopilotKit repo, productionfile_patterns/exclude_patterns/max_file_size):threads-state-lab.tsThe latch is triggered by
//or/*inside a template literal — a URL in a template string, which is common. 32 files were affected; only the ones whose residual chunk crossed the token limit failed hard. The rest degraded silently: one enormous chunk embeds to an averaged, low-precision vector.What changed, and why this design
Fix the cause (
chunking/code.ts): replace strip-then-count with a single left-to-right scan where the quote, template-literal and comment contexts are mutually exclusive. That is the only way//can be classified correctly.Bound the output (
chunking/code.ts): a hard 12,000-character cap on any emitted chunk. The blank-line and mechanical fallbacks bound lines, and nothing bounded characters — a file with no blank lines for hundreds of lines still overflows even with a correct state machine. Splits by lines to stay line-addressable; slices by characters only for a single line that is itself over the cap.Make an over-length input recoverable (
indexing/embeddings.ts): a length-400 is now handled separately from the generic retry. The provider parses theinput[N]the API named, halves that input, and retries — bounded at 12 rounds and a 64-character floor so it always terminates, logging a warning that names the truncation. Any other 400 still throws. No fixed character cap can be right for every tokenizer; converging empirically is what makes an over-length input non-fatal.Bound the retry (
indexing/orchestrator.ts,db/schema.ts,db/queries.ts,types.ts):index_stategains anitem_failuresJSONB ledger — consecutive-failure count, first-failure time, last error, per item id. Three consecutive failing runs quarantines an item: it stops holding the state token, the rest of the source indexes, and every run logs the id, attempt count and error.Three, not one. A network blip, a rate limit and a poison file look identical on their first failure. Quarantining on the first error would trade a visible wedge for invisible data loss.
Quarantine is advisory, not a blocklist. A quarantined item is still handed to the pipeline on every run; a success clears its record outright.
Surface it (
server.ts):/healthand the adminindex-statsop gainquarantined_items—id,attempts,since,error— following thenext_acquireprecedent from Make a reindex actually pick up a change to a source's crawl configuration #160. An item missing from the index is never an invisible gap.Red to green
Three red tests, committed before the fix (bc1e7fc).
RED — chunker (
code-chunker-oversize.test.ts, against unmodified code):RED — embedding provider (
embeddings-oversize-input.test.ts) — the real OpenAI SDK against a real local HTTP server returning production's exact 400 body, with a 20,000-character input that slips under the 30,000-char cap:RED — the wedge (
orchestrator-poison-item-quarantine.test.ts) — three runs, same item failing every time:GREEN — same tests unchanged:
The negative assertion.
does NOT quarantine a TRANSIENT failure — it holds the token and retriesasserts that a one-offECONNRESETkeeps the token attoken-1withstatus: errorandquarantined: false, and that when the item succeeds on the next run the token advances and the failure record is cleared outright — no stale strike carried forward. A third test asserts an existing quarantine clears once the item finally indexes. Without these, the wedge would simply have been traded for silent data loss.Full suite:
3698 passed | 1 skipped, zero failures. (The 4cli.test.tsfailures common in symlinked-node_modulesworktrees are a missingdist/; they pass afternpm run build.) Plusnpm run build,npx tsc --noEmit, andprettier --checkon every touched file.Production state
The
codeindex_state row is left untouched for now, because clearing it cannot recover the source until this lands: the deployed code fails on the same file on every run (most recently at2026-09-11T00:38:42Z), so a cleared row would be back inerrorwithin one cycle. The row to clear after deploy, recorded verbatim:The user-facing impact was narrower than the
last_indexedtimestamp suggests, and the operational cost was larger. Because the source is permanently inerror, every boot re-queues it and — withconfig_fingerprintNULL — takes a full acquire, so 2,664 of the 2,670codechunks were rewritten as recently as2026-09-11 00:39. What was actually stale is the one poison file:threads-state-lab.tsstill carries 3 chunks last indexed2026-08-26. The ongoing costs were a permanent falseerroron/health,codebeing excluded fromaffectedSourceNames(soonReindexCompleteand Atlas cache invalidation never fired for it), and a full re-embed of 1,120 files on every single run instead of an incremental diff. Persisting the fingerprint on the first successful run fixes that last one too.dummy-sourceUnrelated to this bug, and junk.
index_stateid 5311,source_type: html,last_indexed_at: NULL, 0 rows inchunks, and the stringdummy-sourceappears nowhere in this repo's source, config, scripts, or git history. It is not in any source config, soprojectIndexStateForOperatorsfinds no config for it (next_acquire: null) and it is never scheduled. It shares thecoderow's error text only because that message is emitted for any per-item failure. It is harmless but it puts a permanent phantomerroron/health. Recommend deleting that one row — not done here, flagging for a decision.