feat(media,rig,agents,abilities)!: documents — PDF ingress, a documents ability, assets on the run - #120
Conversation
…ts ability, assets on the run Field Note can attach a PDF. The content plane gains a document ingress (PDFium): one text/markdown representation, the PDF retained as source, a facts-only sidecar (sections, page map, per-page counts, page renders as their own image roots). A new first-party ability, `documents`, searches, reads and views those documents; evidence is cited by content address (`attachment://<12 hex>/page/<n>`). media - `createContentIngress` dispatches on bytes: image → normalizer, %PDF- → document ingress. - `pdf.ts` / `pdf-layout.ts`: PDFium codec per document, one shared gate, deterministic coverage under declared bounds; layout from glyph geometry (advance boxes, text-matrix size, baseline grouping, small-caps headings, corroborated titles). Hardened on two real PDFs (Springer chapter, arXiv paper); fixtures + `matrix.pdf` pin the rules. - Sidecar type + `pagesOf`; renders keep the page's aspect ratio. rig - `fitChunks`, `BM25Index`, ranges lifted to rig; `loadDocuments`; content routes gain `GET /v1/media/<m>`, `/config`, `/source`; the registry seeds the store into abilities. - `createReranker` KV default q4_0 → q8_0. Measured on real document windows: at q4_0 ten identical passages spread 4–6 logits across the leaves and the verdict changed sign; at q8_0 the spread is 0.05–0.12 at the same pass time. `reranker-resolution.test.ts` (weights-gated) holds the default to that floor. agents - Assets available to a run: `AgentPoolOptions.attachments`, `ToolContext.attachments` (staged ∪ admitted by any agent), `Source.promptData(attachments)`; tool media may be a descriptor (`TOOL_ATTACHMENTS_KEY`); rail follows bitmaps; `book()` carries roots. - `agent:prefilled` rides the bus beside the `branch:prefill` trace: a host books and shows admissions from the stream it already consumes (announced from `settle()`, since `emit.trace` refuses bus projections). - Exploit mode takes ONE extra pass: `min(tool score, scoreEntailmentBatch)`. BREAKING: `EntailmentScorer.scoreRelevanceBatch` is removed (its only caller). abilities - `documents` (new, 0.1.0): `search_documents` (top-K within a token budget, explore mode always — the attached document is the on-topic universe), `read_document`, `view_page` (projection rule over sidecar facts). Skill: figures by caption, generic names, no silent substitution, failed tools named. - corpus adopts `fitChunks`; web's `fetch_page` says to attach a PDF instead. Gates: npm test (156 files) green, `tsc -p tsconfig.test.json` clean, verify:packed and verify:oci ok, lockfile reconciled; proved end to end on real weights in the scaffold's web target (evidence in the PR).
lloyal-research
left a comment
There was a problem hiding this comment.
Reviewed head 988d7aea142b001dc4eb382228b5dcf6a082c65f against 6641275b3b8690bf7cdb3516d4c99458fee925ab.
The separation of pool-owned asset availability from branch projection is coherent, and the new sibling/prune coverage tests the important ownership rule. I found five actionable issues below; the PDF allocation/input bounds should be addressed before merging.
Validation: npm run build passed. Focused existing suites (documents ability, PDF ingress/render, reranker options) completed with 52 tests passing and the stale q4_0-default assertion failing. Additional local probes reproduced the clipped-figure allocation request (intercepted before allocating), the nested-Form image-check bypass (rendering disabled), the missing delegation argument, and the rejected-page retry suppression through the real pool with its mock native context. No implementation changes were committed or pushed. Real-model scoring/latency claims were not independently rerun; the broader suite also encounters an unavailable Linux native addon in this checkout.
| const [x0, y0, x1, y1] = bbox; | ||
| const width = Math.max(1, Math.round((x1 - x0) * scale)); | ||
| const height = Math.max(1, Math.round((y1 - y0) * scale)); | ||
| const bmp = pdfium.FPDFBitmap_CreateEx(width, height, BITMAP_BGRA, 0, 0); |
There was a problem hiding this comment.
[P1] Bound figure crops independently of the page render
The page's scale does not bound an image object's PDF-space rectangle: an image can extend far outside the page and be clipped by the renderer. Here that entire rectangle becomes a bitmap without clipping or reapplying DEFAULT_MAX_PIXELS / RENDER_MAX_SIDE. I reproduced this with a 100×100-point page containing a valid 1×1-pixel image painted into a 10,000×10,000-point rectangle: the page requested 208×208 pixels, but the figure crop requested 20,833×20,833 = 434,013,889 pixels, versus the 4,194,304 ceiling. I intercepted FPDFBitmap_CreateEx before the large allocation, so no oversized bitmap was allocated. The input-image pixel check passes because the source is only one pixel. This defeats the resident host's memory bound. Intersect crop bounds with the visible page and validate/cap the final crop dimensions before allocating; add a regression fixture with an oversized, clipped image object.
There was a problem hiding this comment.
Fixed in the follow-up commit. Image bounds are clipped to the page as they are read (clipToPage in readPage), an image with no visible part is not a figure candidate, and renderRegionRgba sizes the crop through the same fitScale rule as a page render, so the side and area ceilings hold for crops regardless of the bounds. Regression: clipped.pdf (a 1×1 red image painted into a 2000-pt square on a 100-pt page, hand-written in make.sh) — the figure's bbox is the page, the crop is no larger than the page render and under both ceilings, and its pixels are the image. Red first: bbox came back as [-950, -950, …] and the crop was 4167² px.
| const count = pdfium.FPDFPage_CountObjects(page); | ||
| for (let i = 0; i < count; i++) { | ||
| const obj = pdfium.FPDFPage_GetObject(page, i); | ||
| const type = pdfium.FPDFPageObj_GetType(obj); | ||
| if (type === PAGEOBJ_PATH) pathObjects++; | ||
| if (type !== PAGEOBJ_IMAGE) continue; |
There was a problem hiding this comment.
[P1] Inspect images inside Form XObjects before rendering
This walk only visits top-level page objects and skips every object other than a direct image/path. Images nested inside a Form XObject therefore never reach FPDFImageObj_GetImagePixelSize, although rendering the page will process them. A safe metadata-only probe (maxRenderedPages: 0) refused a direct 10,001×10,000 image with the pixel-ceiling error, but accepted the same image wrapped in a Form and recorded imageObjects: 0. This bypasses the decompression guard; it also makes ordinary graphics inside Forms invisible to figure discovery and the ability's projection rule on text-bearing pages. Traverse Form children with bounded traversal and the appropriate transforms, applying the image-size check to nested images before any rendering. Test the same image both directly and nested.
There was a problem hiding this comment.
Fixed in the follow-up commit. readPage now walks Form XObjects (FPDFFormObj_CountObjects / FPDFFormObj_GetObject) with a depth bound of 8 and a visit cap of 20,000 objects; nested images go through the same pixel-ceiling check and count toward imageObjects and figure discovery, with their bounds carried into page space through the composed form matrices (FPDFPageObj_GetMatrix per ancestor). Fixtures: form.pdf (a 2×2 green image in a form with /Matrix translate (10,10) under an outer translate (40,40) → bbox [50,50]–[150,150], crop pixels green), bigimage.pdf and bigform.pdf (a 10001×10000 image direct and nested: both refused with the ceiling error under maxRenderedPages: 0). Red first: imageObjects was 0 and the nested oversize resolved.
| trace: opts.trace, | ||
| policy: opts.policy, | ||
| scorer: opts.scorer, | ||
| attachments: opts.attachments, |
There was a problem hiding this comment.
[P2] Forward current assets through the existing delegation caller
This new input is wired through agentPool, but packages/rig/src/tools/delegate.ts:208-215 still calls it with static poolOpts, parent: context?.branch, and scorer: context?.scorer only. It never passes context.attachments. Consequently a delegated child inherits the parent's attention but starts with an empty document asset set, so search_documents / read_document report no attached documents. A focused test executing the real DelegateTool with a populated ToolContext.attachments and capturing its agentPool options receives attachments: undefined. Forward the current call's assets at that boundary (not merely the initial static pool configuration), and cover delegation after a document was admitted during the outer run.
There was a problem hiding this comment.
Fixed in the follow-up commit. DelegateTool forwards attachments: context?.attachments into agentPool, which at that call is the run's staged roots plus everything any agent admitted so far. Test: packages/rig/test/delegate-assets.test.ts executes the real tool with a populated ToolContext.attachments (a staged root and a mid-run admitted root) under a mocked agentPool and reads the options it received. Red first: attachments arrived undefined.
|
|
||
| if (!projectable(page)) return { ...where, note: `Page ${page.page} is text only — read_document gives you its text.` }; | ||
| if (!page.render) return { ...where, note: `Page ${page.page} is not archived as an image.` }; | ||
| this._viewed.add(key); |
There was a problem hiding this comment.
[P2] Record viewed/read evidence only after successful admission
_viewed.add(key) runs when the tool returns the descriptor, before the scheduler decides whether the image can land. A settle rejection can replace that result with a nudge while leaving the same agent alive; its next call now returns Already viewed even though the model never saw the page. I reproduced this through the real pool with the existing mock native context: first image priced above headroom → one settle-reject nudge → capacity made available → same page requested again. There were two tool calls and zero multimodal prefills because the retry was suppressed. The figure path has the same eager write, and read_document similarly updates _read before its text lands. Base this deduplication on successfully admitted evidence, or stop suppressing results merely because a tool produced them. Add an admission-rejection/retry scenario, not just two direct tool calls.
There was a problem hiding this comment.
Agreed on the invariant: evidence bookkeeping must follow admission, and a tool cannot observe admission. For view_page the fix stops suppressing: a repeat by the same agent carries the page (or figure) root again with a note that it was viewed before, so a settle rejection followed by a retry still lands the page, and admission stays the only gate on cost — which is the plan's own rule for this tool. Test updated: the second call carries the descriptor and the note.
read_document and corpus read_file share rig's unread-range convention, which has the same eager write; changing it means giving tools the agent's admitted evidence through ToolContext (a core capability, not an ability patch), so I have left both as they are and recorded the follow-up in the plan rather than diverge two abilities. The admission-rejection scenario through the real pool belongs with that change.
There was a problem hiding this comment.
Correcting my second paragraph above: no new capability was needed, and leaving the two read tools as they were was the wrong call.
CallingAgent already carried everything the tools were missing. The pool sets it on every dispatch (execute.ts), and it holds the agent, its branch, and its booked history lineage-wide through walkAncestors. What the three tools were reading instead was ToolContext.agentId, a duplicate added before CallingAgent existed and never retired. An id on the port is what invited each tool to keep private per-agent state, which is how all three came to record evidence before the pool had admitted it. So this is one defect with three instances, not one ability patch plus a platform follow-up.
The shape the fix takes:
ToolHistoryEntrygainsoutcome, taken from the prefill item's ownkind. This was a real hole underneath the finding: a settle nudge's replacement item carries the ORIGINAL call's name and arguments, sotoolHistoryclaimed a nudged call had landed. Any "already seen this" guard reading history alone would have inherited the same blindness the private maps had.landedCalls(agent, tool)in agents: one pure walk of the agent's booked history, filtered to what actually landed.- All three tools delete their maps and read that.
read_documentand corpusread_filesubtract the ranges that landed;view_pageasks membership and still returns the page either way, with the note now following admission rather than the tool's memory. ToolContextlosesagentIdandbranch. Who is calling is ambient; the port carries the values of the call.
The settle-reject-then-retry case is now tested through the real pool rather than a stub: the page defers over headroom, the nudge lands, the retry lands the page, and the nudged call is absent from landedCalls.
| typeK: opts?.typeK ?? 'q8_0', | ||
| typeV: opts?.typeV ?? 'q8_0', |
There was a problem hiding this comment.
[P2] Update the existing default-KV test with this default change
The new q8_0 default leaves packages/rig/test/reranker-options.test.ts:70-75 asserting q4_0 for both types. On this head, npx vitest run packages/rig/test/reranker-options.test.ts --maxWorkers=1 --pool=threads fails with expected 'q8_0' to be 'q4_0'; this uses mocks and does not require weights or a native backend. Keep the intended q8_0 runtime change, update the old test/title to that contract, and rerun the gate. The weights-gated resolution test does not replace this non-skipped unit assertion, so the PR's current all-green gate statement is not reproducible.
There was a problem hiding this comment.
You are right, and this also corrects the PR's gate statement: the one failure in the full run was this test, not the one I attributed it to. reranker-options.test.ts now states the q8_0 contract (title and both assertions) and the rig suite runs green with it; the full-suite gate has been rerun on the new head.
…nd a test that was red first - media: image bounds are clipped to the page as they are read and a crop is sized by the same `fitScale` rule as a page, so the side and area ceilings hold for crops (an image painted far past the page no longer asks for a 20,000-pixel bitmap); `readPage` walks Form XObjects with a depth bound and a visit cap, applying the pixel-ceiling check to nested images and carrying their bounds into page space through the composed form matrices. Fixtures `clipped.pdf`, `form.pdf`, `bigimage.pdf`, `bigform.pdf` (hand-written in make.sh) pin all three. - rig: `DelegateTool` forwards the delegating call's `attachments` into the child pool — the run's staged roots plus everything admitted so far — so a delegated agent sees the documents; `delegate-assets.test.ts`. - documents: a repeated `view_page` carries the page again with a note. The tool cannot see whether its last result landed (a settle nudge may have replaced it), so suppression left the model blind; admission is the gate. - rig: `reranker-options.test.ts` states the q8_0 default contract; the earlier gate statement had misattributed this file's failure.
lloyal-research
left a comment
There was a problem hiding this comment.
Follow-up while reviewing stacked #121: the crop, shallow Form XObject, delegate-assets and reranker-option regression suites pass, but the new Form traversal still allows the original pixel-ceiling check to be bypassed when its inspection limit is reached. This finding belongs to #120's latest commit, not #121's delta.
| const type = pdfium.FPDFPageObj_GetType(obj); | ||
| if (type === PAGEOBJ_PATH) { pathObjects++; return; } | ||
| if (type === PAGEOBJ_FORM) { | ||
| if (depth >= MAX_FORM_DEPTH) return; |
There was a problem hiding this comment.
[P1] Refuse uninspected content when the Form traversal limit is reached
Both traversal limits return as if validation succeeded. That bounds this inspector but does not stop PDFium from subsequently rendering the uninspected objects. I reproduced this safely with rendering disabled (maxRenderedPages: 0): the same 10001×10000 image is rejected at Form depths 0, 1 and 8, but is accepted at depth 9, with the sidecar reporting imageObjects: 0. The original oversized-image guard is therefore still bypassable; the object-count early return has the same fail-open shape.
If inspection cannot complete within either limit, reject the document/page before rendering (or explicitly prevent rendering the uninspected content). Add a depth-limit regression alongside the existing bigform.pdf case. The probe did not decode or allocate the oversized bitmap.
There was a problem hiding this comment.
Confirmed, and tracked as #122 rather than fixed here.
Verified in source: the ceiling at :376 only fires on images the walk reaches, so a return at the limit accepts content that rendering then decodes. The comment two lines above the walk already states the requirement it breaks — "rendering will decode what they hold, so the check and the counts must see it too."
There are three sites, not one. :361 is the Form depth you found, :357 is the walked-object cap with the identical shape, and :373 is the same pattern one level in: the ceiling check sits inside if (FPDFImageObj_GetImagePixelSize(...)), so an image whose dimensions cannot be read skips it with no nesting required. Fixing only the traversal limits moves the target to the probe.
Two things the fix does not need to solve, checked rather than assumed. Propagation already works: the ingest loop at :683 has no per-page try/catch, so a throw exits the page loop, the document scope and ingestion — and it runs before layoutDocument and before the renders map is built, so a refused page cannot reach a render call by construction. Cleanup already works: the existing ceiling throw at :376 exits through the same try and the walk's finally frees matrixBuf.
One trade recorded in the issue rather than decided silently. Depth 8 costs nothing real. MAX_WALKED_OBJECTS = 20_000 is a complexity cap a dense vector page can reach honestly, so throwing there turns a legitimate document into a hard rejection; the message needs to say the page could not be fully inspected.
Accepted for the alpha.4 cut with that known, since this PR does not regress it and the alpha channel makes a later fix a republish plus a re-vendor rather than a patch.
What
Field Note can attach a PDF. Three ordered packages, each green before the next, plus the review round that followed the first real use:
fitChunks,BM25Index, ranges move to rig; corpus adopts windowed chunks.createContentIngressdispatching on bytes; content routes for manifest / config / source; assets available to a run (attachmentsstaged into the pool, grown by any agent's admission, read per tool call); tool media as descriptors; rail follows bitmaps.documentsability + the seam —search_documents/read_document/view_page, citations by content address, the web ability's PDF message.Review-driven, after the first daily-driver use (five asks on a real pharmacology chapter):
createRerankerKV default q4_0 → q8_0. Ten identical passages in one call, real document windows: at q4_0 the scores spread 4–6 logits across the leaves and the verdict changed sign (the answer row scored −5.5…−1.7, the reference list −0.2…+4.2); at q8_0 +5.2 (spread 0.07) and −7.7, same pass time. A weights-gated resolution test holds the default to that floor. Re-running the same four questions on q8_0: the answer-bearing window was the first search's feat(agent-tools): pass agent Id for deduping and update docs #1–Feat/recursion #3 in three of four; the fourth needs the generic name.agent:prefilledon the bus — admissions reach the host from the stream it already consumes (run dir + UI), no trace tee.search_documentsselects top-K within a token budget, in explore mode — the reranker is a relative judge; the global floor returned nothing in 12 of 13 searches, and exploit's min() with the original question vetoed the passage a good query found.min(tool score, original-question score));scoreRelevanceBatchis removed — BREAKING for scorer implementers (it had one caller).Evidence (real weights, scaffold web target, Qwen3.5-4B + Qwen3-Reranker-0.6B)
# document_research — available files.search_documents→bm25:*/rerank:*→ hits withdocument/id/pageStart/pageEnd/cite;read_documentby page;view_pagereturns the page root as a DESCRIPTOR (ingress untouched) and lands on the agent's branch as onebranch:prefill{role:toolResult, attachments:[…]}at 1,629 cells; the exchange's meta line carries the admitted root; reload/reopen restores the card; the lightbox opens the archived page render by digest and the original PDF through/source.view_page(7)(was the wrong figure).Gates
npm test156 files green ·tsc -p tsconfig.test.jsonclean ·verify:packedok ·verify:ociok · lockfile reconciled (no change) · agents 85 files / 455 · media 14 files / 121 · documents ability 6 files / 28.No CI runs on
feat/mtmd(ci.yml = main only); the Linux runner meets this at the next merge to main, which is why no ability test touches the native addon.Follow-ups (not in this PR)
lloyal publishoflloyal/documents@0.1.0(reviewer approval); the CLI template port is feat(research,cli): documents — the template port, the default ability, media's optional peers lloyal-ai#27 (DEFAULT_ABILITIES.research+= documents), which merges after the approval.fetch_pagePDF door.