Skip to content

api: raise the docs page bound and make truncation visible - #810

Open
ben-dz wants to merge 3 commits into
mainfrom
bdz/docsfetch-page-bound
Open

api: raise the docs page bound and make truncation visible#810
ben-dz wants to merge 3 commits into
mainfrom
bdz/docsfetch-page-bound

Conversation

@ben-dz

@ben-dz ben-dz commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

docsfetch cut every MCP-served docs page at MaxPageBytes = 10000. Measured against malbeclabs/docs@751d45b, that truncates 12 of the 27 MCP-reachable slugs:

bytes page bytes page
33,856 contribute-provisioning 12,200 Swapping-sol-to-2z
21,551 troubleshooting 11,789 geolocation
17,551 contribute 11,693 glossary
14,975 contribute-ops-management 11,086 kalshi-runbook
13,694 contribute-operations 10,880 setup
12,397 kalshi 10,648 multicast-admin

The bound arrived in #770 as a defensive constant, not a measured one. It was also silent — a bare ... (truncated) appended to the content string and no signal anywhere else — which is why this went unnoticed. For get_onboarding_runbook it means an agent walks a user through a procedure with the tail of the runbook missing.

Raises the bound to 64 KiB (~2x the largest real page), makes the marker name the byte count and the source URL so an agent can point the user at the rest, and reports truncated on both tool outputs.

Three things worth a reviewer's attention:

The chat agent had its own copy of the fetch, still bounded at 10,000 with the old silent marker, so the raised bound would not have reached web or Slack users. readDocs in agent/pkg/workflow/v3/pipeline.go now calls docsfetch.Client.Read — a net −55 lines, and it picks up two things it never had: the ValidPage slug guard (it previously interpolated the page straight into a raw.githubusercontent URL) and a request timeout.

A truncated runbook index is a hard error, not a short catalog. loadRunbookCatalog parses runbooks.md into the list of known services; a cut index silently drops entries, and get_onboarding_runbook then answers unknown runbook "kalshi" for a runbook that exists. That is a confident wrong answer rather than a degraded one, so it fails instead. It is the only Read call site whose result the handler branches on.

All three truncation cases now log at WARN. The in-band marker reaches the model but not an operator; the index refusal takes the tool down for every caller, so it is the one that most needed a line.

read_docs also advertises three pages that no longer exist and one that is an HTML redirect stub — in both the MCP and agent tool descriptions. Real, but a separate change.

Testing Verification

TestRead_LargestRealPageIsNotTruncated pins a 34,000-byte page reading back whole; red-checked against the old constant, where it fails. The marker's wording is asserted in docsfetch, which runs without containers.

No test here can catch a docs page crossing the bound — nothing in lake knows those sizes, they are fetched at runtime from another repo. Loud truncation is the mitigation chosen over a cross-repo CI size check.

Two gaps, both environmental: the four new api/handlers tests and the eval suite were not executed locally, since both packages' TestMain starts containers and Docker was unavailable. Both compile and vet clean. No eval references read_docs, so the agent change is unexercised by that suite either way; agent/pkg/workflow/v3 unit tests pass.

make lint and go build ./... fail identically on main (dev/controlcenter needs a built UI bundle). Both clean when scoped to the touched packages.

Blocks

malbeclabs/docs#200 — its new ## Teardown section lands past the old cut on kalshi-runbook.md (11,086 bytes). The alternative offered there, trimming the page under 10,000, is not the fix taken.

ben-dz added 2 commits August 28, 2026 01:38
docsfetch cut every MCP-served page at 10,000 bytes, which truncates 12 of
the 27 reachable docs slugs — including the kalshi runbook, so an agent
driving it loses the tail of the procedure. The cut was silent: a bare
marker in the content string and no signal anywhere else.

Raise the bound to 64 KiB (largest real page is ~34 KB) and report
truncation as data on both tool outputs, with the marker naming the source
so an agent can point the user at the rest.
A cut runbook index yields a short catalog, so get_onboarding_runbook would
answer "unknown runbook" for a runbook that exists. Refuse it instead.

Also log truncation at WARN, since the in-band marker is visible to the
model but not to an operator.
@ben-dz
ben-dz requested a review from nikw9944 August 28, 2026 01:49
@ben-dz
ben-dz marked this pull request as ready for review August 28, 2026 01:49
@ben-dz
ben-dz requested a review from a team August 28, 2026 01:49

@nikw9944 nikw9944 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. Two things worth folding in: the chat agent keeps its own docs fetch still bounded at 10,000 bytes with the old silent marker, so the pages this PR unblocks stay truncated on the web and Slack paths; and the new truncated-index refusal takes get_onboarding_runbook down for every caller while logging nothing, making it the one truncation case with no operator signal.

Comment thread utils/pkg/docsfetch/docsfetch.go
Comment thread api/handlers/mcp_onboarding.go
The agent had its own copy of the docs fetch with the old 10,000-byte bound,
so the pages this branch unblocks stayed truncated on the web and Slack
paths. Routing it through docsfetch.Client.Read collapses the duplicate and
picks up the slug validation it never had.

The truncated-index refusal was the one truncation case with no operator
signal; log it at WARN like the other two.
@ben-dz

ben-dz commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Both folded in — thanks, the agent copy was a real miss.

The duplicate fetch. Routed readDocs through docsfetch.Client.Read. It was a full duplicate of that function (same base URL constant, same request/status/read/truncate sequence), so it collapses to a net −55 lines and io/net/http/errors all drop out of the file. Two things it picks up beyond the raised bound: the ValidPage slug guard — it previously interpolated the page name straight into a raw.githubusercontent URL with no validation — and a request timeout, since it was on http.DefaultClient. Error strings are unchanged; docsfetch already used the same wording. I checked every slug in the agent's tool description against ValidPage and they all pass, so no working path narrows.

The index refusal. Now logs WARN before returning, so all three truncation cases have a line. I kept the refusal itself: runbooks.md is 837 bytes against a 64 KiB bound, so it is a guard that should never fire, and a short catalog answering unknown runbook "kalshi" for a runbook that exists seemed worse than a loud failure.

One thing I could not verify: the four new api/handlers tests and the eval suite both need Docker, which my environment does not have, so neither ran locally. They compile and vet clean. No eval touches read_docs, so the agent change is unexercised there regardless — CI is the first real run of the handler tests.

@ben-dz
ben-dz enabled auto-merge (squash) August 28, 2026 18:05
@ben-dz
ben-dz disabled auto-merge August 28, 2026 18:06
@ben-dz
ben-dz enabled auto-merge (squash) August 28, 2026 18:06

@nikw9944 nikw9944 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both round-1 findings verified fixed in 814bb2c: the agent's duplicate docs fetch now routes through docsfetch.Client.Read (one bound, one marker, error wording preserved), and the truncated-index refusal logs WARN before returning. Nothing new in that commit. lgtm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants