Skip to content

feat(shell): documents, records, and the primitives four verticals hand-rolled - #380

Merged
drewstone merged 2 commits into
mainfrom
feat/vertical-uplift
Aug 4, 2026
Merged

feat(shell): documents, records, and the primitives four verticals hand-rolled#380
drewstone merged 2 commits into
mainfrom
feat/vertical-uplift

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

Every module here was extracted from working vertical code, not invented. An audit of tax-agent and legal-agent found the same capabilities implemented independently, and one agent-app defect blocking a product outright.

What the audit found, and what this fixes

Finding This PR
DOCX rejected with 415 — the attachment allow-list admits images + PDF only, so legal-agent's contract proposer declares DOCX support that can never run Real OOXML sniffing: identified by the OPC content-types part, so a renamed .zip is still refused. Word/Excel/PowerPoint in; macro-enabled packages need explicit opt-in
tax + legal each integrated the same PDF wasm /documents + /documents/pdf-inspector
tax + legal + relationships each hand-rolled a source-cited, supersedable row store /record + /record/drizzle
four verticals copied an editable grid RecordGrid (/web-react)
dozens of screens render a failed fetch as empty data; one showed "Saved" on a 404 /web-react/asyncsucceeded is reachable only through a branded confirmation, so that bug is unrepresentable
agent-authored UI is inert — no input nodes, and onAction never passed /openui + /openui-react, with a test that mechanically forbids a model call on the action path

Measured, not assumed

The crew probed the real wasm before designing, and found a defect both verticals shipped: extractText panics on any page with no text layer — including a mixed document whose other pages are readable — so a 40-page contract with one scanned exhibit was rejected outright. Partially scanned documents now extract their readable pages and name the ones they couldn't. Also measured: classifyPdf reports OCR pages 0-indexed while detectPdf reports 1-indexed (both verticals carried a hand-written p + 1), and processPdf's own pagesNeedingOcr flagged both pages of a fully text-based PDF, so classification always comes from detectPdf. All of it is pinned by tests so an engine bump can't silently invalidate the control flow.

DOCX is a dependency-free reader over DecompressionStream('deflate-raw') rather than mammoth — mammoth pulls 10 transitive deps whose CJS entry needs fs and is unproven under workerd. Adopting this removes Node-shaped dependencies from a Worker bundle rather than adding them.

Compatibility

Additive only: no existing signature, export, or behavior changes. New capability arrives as new subpaths, wired into tsup/exports/knip per the repo's module rule.

Verification

  • pnpm typecheck clean · pnpm test 4452 passed / 10 skipped.
  • pnpm build: the ESM build succeeds; the DTS step OOMs — and it does so identically on unmodified origin/main (verified, with 96 GB free on the host), so it is a pre-existing worker resource cap in tsup's declaration build, not a regression from this PR. Worth its own fix.

🤖 Generated with Claude Code

…tives four verticals hand-rolled

Adds /documents (+/documents/pdf-inspector), /record (+/record/drizzle),
/openui (+/openui-react), /legibility-adjacent web-react primitives —
RecordGrid and the three-state async contract — and widens the
attachment gate to OOXML.

Each was extracted from working vertical code, not invented: two
products had each integrated the same PDF wasm, three had hand-rolled a
source-cited supersedable row store, four had copied an editable grid,
and dozens of screens rendered a failed fetch as empty data. Measuring
the real PDF engine found a defect both verticals shipped — extractText
panics on any page without a text layer, so a contract with one scanned
exhibit was rejected outright; partially scanned documents now extract
their readable pages. DOCX reads through DecompressionStream rather than
mammoth, so adopting this removes Node-shaped transitive dependencies
from a Worker bundle instead of adding them.

Word, Excel and PowerPoint now pass the upload gate — identified by the
OPC content-types part, so a renamed .zip is still refused, and
macro-enabled packages need an explicit opt-in.

Every change is additive: no existing signature, export, or behavior
changes for a pinned consumer.

@tangletools tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Auto-approved drewstone PR — e4a721bf

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: drewstone_author · 2026-08-04T07:14:20Z

@tangletools tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Value Audit — sound

Verdict sound
Concerns 2 (1 low, 1 weak-concern)
Heuristic 0.1s
Duplication 0.0s
Interrogation 322.4s (2 bridge agents)
Total 322.5s

💰 Value — sound

A large, additive consolidation that extracts shell mechanism 2-4 vertical products had hand-rolled independently (document extraction, source-cited records, editable grid, async-state contract, OpenUI host-half) plus widens the attachment gate to OOXML — each module follows the framework's document

  • What it does: Adds six new additive subpaths to the @tangle-network/agent-app shell: (1) /documents + /documents/pdf-inspector — PDF/DOCX/text extraction behind a PdfEngine wasm seam, with measured handling of the engine's extractText panic on textless pages and partially-scanned docs; (2) /record + /record/drizzle — a source-cited, supersedable, reviewable entry store (pure leaf + drizzle store) encodi
  • Goals it achieves: Centralize shell mechanism that multiple products (tax-agent, legal-agent, relationships-agent, creative-agent) had each built separately, so a reference consumer is 100% on agent-app for shell mechanism and stops fork-duplicating. Fix one concrete product blocker (DOCX rejected with 415 because the allow-list admitted images+PDF only). Fix one measured defect both verticals shipped (extractText p
  • Assessment: Strongly in the grain of the codebase. The framework's governing 'one rule' (AGENTS.md) — a capability that makes sense without a specific app's tool sidechannel belongs in the shell — is satisfied by every module here: extraction, records, grids, async contracts, and the OpenUI host-half are all substrate-free shell mechanism. Every documented invariant is honored: engines/wasm/schema libs are OP
  • Better / existing approach: Checked the obvious reuse candidates and none apply. (1) MissionStorePort (src/missions/service.ts:148) governs mission lifecycle — status machine, cursor, plan, budget ledger, audit events — a different abstraction than a source-cited supersedable entry store with review states + fold rules + conflict detection; the AGENTS.md explicitly documents /record as 'the /missions store-port PATTERN, no
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error

🎯 Usefulness — sound

Six extracted-from-production modules each fill a real gap multiple fleet apps hand-rolled, all wired correctly, none duplicate an existing pattern, with one genuinely live behavior fix (OOXML upload gate) and one live opt-in integration (openui → render_ui).

  • Integration: Three surfaces are LIVE in this PR: (1) the OOXML widening — ATTACHMENT_ACCEPT now admits .docx/.xlsx/.pptx and binary-sniff identifies them by the OPC content-types part, exercised through the chat-routes upload path (src/chat-routes/binary-sniff.ts, attachment-validation.ts); (2) /openui augments the existing render_ui tool via the opt-in interactiveUi flag in buildAppToolOpenAITools (src/tools/
  • Fit with existing patterns: No duplication and no competing pattern found anywhere. /record is a new DOMAIN over the established CONCRETE-FACTORY pattern (matches src/intakes/drizzle.ts, src/sequences/drizzle-store.ts, src/teams/drizzle.ts), distinct from the interface-only port pattern (WorkProductStorePort at src/work-product/types.ts:280-296, MissionStorePort at src/missions/service.ts:148-157); its extraColumns+requireEx
  • Real-world viability: The designs address the documented real-world failure modes rather than only the happy path: the record store encodes the SQLite head-race + NULL-dedup + second-resolution-ordering invariants in schema and store with tests proving both directions; the rate limiter fails closed on poisoned KV state and the free-route-limit refuses identity-less requests and KV throws; /documents handles the measure
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: magic number added src/documents/extract.ts

    • deflate reaches ~1000:1 on repetitive input, so capping the upload does

💰 Value Audit

🟡 OOXML mime literals duplicated across two import-free leaves [duplication] ``

The OOXML mime constants are defined twice: OOXML_WORD_MIME at src/chat-routes/binary-sniff.ts:26 and DOCX_MEDIA_TYPE at src/documents/media-type.ts:12 are the same string 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' (same for the Excel/PowerPoint/macro-enabled variants). This is NOT a duplicated capability — binary-sniff is the upload GATE sniffing untrusted bytes (browser-safe, import-free per its header) and media-type is the extractor ROUTING layer (import-free l


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260804T072628Z

@drewstone

Copy link
Copy Markdown
Contributor Author

Correction on the build note in the description: the DTS OOM is not an unexplained pre-existing failure. The repo's CI documents 12288 as the supported heap (8192 is a known-bad value, which is what I used). Verified on this branch:

NODE_OPTIONS="--max-old-space-size=12288" pnpm build   # 0 errors

So: typecheck clean, 4452 tests passing, and the build is green at the documented heap. No build regression from this PR.

@drewstone
drewstone merged commit 1b771e1 into main Aug 4, 2026
1 check passed
@drewstone
drewstone deleted the feat/vertical-uplift branch August 4, 2026 07:42
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.

2 participants