ENG-3697: Bound Live and Get responses, and stop List deleting its own content - #23
Conversation
…n content Simplify built each row's `text` from `summary`/`highlights` and dropped the hyperdoc tree. Only the query path returns `ScoredDocumentResponse`, the sole model carrying those fields (schemas.py:152). `/memories/list`, `/memories/get/*` and every `/live/*` route return the plain `DocumentResponse` (schemas.py:75) — no summary, no highlights, body only in the tree. Two consequences, in opposite directions: - `Document → List` had Simplify wired with the default on, so since 0.7.0 it has returned every row with `text: ""` and the body dropped. Metadata-only rows; the content was deleted outright. - `Document → Get` and all three document-shaped `Live` operations had no bounding at all, so they emitted the full tree. The Live resource is the surface built for an AI-agent node, which made it both the most likely to feed a model and the only one shipping an unbounded payload. `matchedText()` now falls back to a depth-first flatten of the tree, capped at 2000 chars to mirror the API's own per-highlight cap, and the toggle is wired to the four operations that lacked it. `Live → List Sources` is excluded on purpose: capability descriptors, no tree to bound. Six existing tests failed against this and are updated deliberately. Five called the unwrappers with `this === undefined` — safe only while those functions read no parameters; n8n always binds a real context. The sixth asserted `text === ''` for a document with no summary or highlights, which encoded the bug; it now asserts the tree fallback, with a separate test that text is empty only when the document genuinely has no text anywhere. Verified: build, lint, 92/92 tests (was 82). A 200KB live document goes from 200,155 bytes to under 2,500. Not yet exercised inside a real n8n instance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
EntelligenceAI PR SummaryThis PR bounds document payloads across Live search/get/list and Document get responses, with simplification enabled by default and an opt-out for callers that need the full hyperdoc tree. Simplification now falls back to depth-first hyperdoc flattening when summaries or highlights are absent, preserves structured leaf fields, and caps generated text at 2,000 characters, preventing List and Live responses from losing their content while avoiding unbounded payloads. Live Search, Get, and List now share Connection ID handling that trims values, omits empty inputs, and rejects non-UUIDs before requests reach the API, with clearer guidance for AI Agent usage. Added regression coverage verifies payload bounding, content preservation, single-document handling, cursor/envelope behavior, and connection validation. flowchart TD
LiveOps["Live Search / Get / List"] --> Connection["Shared Connection ID property"]
Connection --> Validate["preSend UUID validation"]
Validate --> LiveAPI["Live API"]
LiveAPI --> LiveBound["Bound Live output"]
DocGet["Document Get"] --> DocBound["simplifyOne postReceive"]
DocAPI["Document List / Get responses"] --> Simplify["simplifyDocument"]
Simplify --> Fallback["Flatten hyperdoc tree and cap text"]
Fallback --> BoundOutput["Simplified bounded item"]
LiveBound --> Simplify
DocBound --> Simplify
classDef newBehavior fill:#dcfce7,stroke:#16a34a,color:#14532d;
class LiveOps,Connection,Validate,LiveAPI,LiveBound,DocGet,DocBound,DocAPI,Simplify,Fallback,BoundOutput newBehavior;
Review Scorecard
Issues found:
Fix before merge but low risk because the blast radius is contained to the changed document-simplification paths. The main correctness issue is that Evaluated against
Files requiring special attention
|
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
| function matchedText(document: HyperspellDocument): string { | ||
| if (typeof document.summary === 'string' && document.summary.length > 0) { | ||
| return document.summary; | ||
| } | ||
| const highlights = Array.isArray(document.highlights) ? document.highlights : []; | ||
| return highlights | ||
| const fromHighlights = highlights | ||
| .map((highlight) => highlight?.text) | ||
| .filter((text): text is string => typeof text === 'string' && text.length > 0) | ||
| .join('\n\n'); | ||
| if (fromHighlights.length > 0) return fromHighlights; | ||
| return flattenHyperdoc(document.document, MAX_SIMPLIFIED_TEXT); |
There was a problem hiding this comment.
Search and Answer still pass through summary verbatim or join every highlight without applying MAX_SIMPLIFIED_TEXT. Their callers in search/index.ts therefore can emit multi-kilobyte simplified rows, so the response bound only works for tree-fallback documents.
The fixture backing the List regression was hand-built from reading schemas.py, and named PROD_LIST_RESPONSE in a file whose convention reserves that framing for verbatim recordings — the existing fixtures say so explicitly, warning that hand-rolled JSON lets "backend schema drift hide behind an idealized fixture". Mine was exactly that, and the reasoning was circular: read DocumentResponse, build a DocumentResponse-shaped object, prove the code mishandles DocumentResponse. Now RECORDED_LIST_RESPONSE, verbatim `GET /memories/list?size=2` bytes from a local core (app 1 / hyperdev) on 2026-08-12, captured after adding one document through POST /memories/add. The recording independently confirms the premise this whole change rests on — item keys are exactly resource_id, source, type, title, status, collection, metadata, ingested_at, last_modified_at, document_date, document, with no summary, no highlights and no score, and the body only inside the tree. Adds a fixture guard test so that if core ever puts summary/highlights on the list path, the suite fails loudly rather than the fallback quietly becoming dead code. Assertions now read identity fields off the fixture instead of hardcoding literals. Two assertions changed with the fixture: the recorded document is `pending`, not `completed` as the invented one claimed. 93/93 pass, lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
End-to-end verification inside a real n8n instancePreviously this PR was only backed by unit tests against the built Control vs. fix — same workflow, same API, same document
The published version returns the row with its content deleted. This branch returns the body. Both runs used an identical workflow and the same Emitted row keys on this branch: Tree dropped, envelope preserved, Fixture is now recorded, not inventedThe fixture backing the List tests was hand-built from reading Replaced with 93/93 tests pass, lint clean. Still not covered
Incidental finding, unrelated to this PR
|
…ping a 502
Reported from IHQ's instance on n8n 2.33.4 Cloud: Live → Search failing with
`502 "Upstream source error."`, raised by our own raiseApiErrors.
That message is wrong in a way that costs the workflow author real time — it
blames the provider, when no provider was ever contacted. Prod logs show the
actual cause:
Live source call failed: DBAPIError(asyncpg.exceptions.DataError:
invalid input for query argument $4: 'linear'
(invalid UUID 'linear': length must be between 32..36 characters, got 6))
`connection_id` is arriving as a source name. The node sets usableAsTool, so
an AI Agent fills the parameters from their descriptions, and the old wording
— "Specific connection ID when the user has multiple connections for this
source" — reads to a model as an invitation to name the source. Prod saw
exactly that: 'linear', 'github', 'google_drive'. Core then puts the string
into a UUID column (live_access.py `stmt.where(Connection.id ==
connection_id)`, parameter $4), asyncpg rejects it, and a blanket
`except Exception` in api/live.py maps it to the generic 502.
Reproduced against a local core, confirming it is the parameter and not the
source being unavailable:
POST /live/linear/search {"query":"test"}
-> 400 "No linear connection is accessible to this user." (clear)
POST /live/linear/search {"query":"test","connection_id":"linear"}
-> 502 "Upstream source error." (opaque)
Core should return a 400 here, and that is tracked separately — but the node
should not send a malformed request in the first place, and a guard here keeps
working against every already-deployed core version.
All three document-shaped Live operations shared this field by copy-paste, on
two different transports (Search posts a body; Get and List Resources use the
query string). They now share one definition:
* preSend rejects a non-UUID with a message naming the likely mistake.
Deliberately not a silent drop — silently ignoring it would make a request
scoped to the wrong connection look like it succeeded.
* The description states the format and the default before the purpose,
since that text is the model's only input when running as a tool.
Verified end-to-end in n8n 2.34.5 against a local core. The same workflow that
produced `502 Upstream source error.` now fails in the node with:
Connection ID must be a connection UUID, not "linear".
99/99 tests pass (was 93), lint clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Added: the Live 502 reported from IHQ's instanceScope grew by one commit ( It is not a provider outage, and not user error. Prod logs give the real cause, which the 502 hides:
Core then puts the string into a UUID column ( Reproduced against a local coreConfirms it is the parameter, not the source being unavailable:
The changeAll three document-shaped Live operations had this field by copy-paste, across two transports (Search posts a body; Get and List Resources use the query string). They now share one definition with:
Verified end-to-endn8n 2.34.5 against a local core. The same workflow that produced
99/99 tests pass (was 93), lint clean. Needs a companion fix in core — not in this PRThe API should return 400 for a malformed
Different repo, so it needs its own PR. |
| for (const prop of [body, query]) { | ||
| assert.equal(prop.name, 'connection_id'); | ||
| assert.equal(prop.routing.send.property, 'connection_id'); | ||
| assert.equal(prop.routing.send.preSend.length, 1); |
There was a problem hiding this comment.
This only checks that one preSend entry exists, not that it is validateConnectionId. The direct validation tests can pass while connectionIdProperty wires an unrelated no-op guard, allowing malformed IDs through the shared property.
| assert.equal(prop.routing.send.preSend.length, 1); | |
| assert.deepEqual(prop.routing.send.preSend, [validateConnectionId]); |
Prompt to fix with AI
Copy this prompt into your AI coding assistant to fix this issue.
In tests/connection-id.test.mjs:86, replace the length-only assertion with an assertion that `prop.routing.send.preSend` exactly equals `[validateConnectionId]`, so the shared property test fails if the validator is replaced or omitted.
… column The preSend guard trims before validating, so " " is accepted as "not provided" — but the routing expression did not trim, and " " is truthy in JS. The node therefore shipped it, core put it in the UUID-typed Connection.id column, asyncpg raised DataError, and the caller got back `502 "Upstream source error."`: the precise failure this field exists to prevent, still reachable through the guard meant to stop it. Reproduced end to end against a pre-fix core, then confirmed fixed. Trimming on send also rescues a pasted " <uuid> ". Core parses the value with UUID(), which does not tolerate surrounding whitespace, so the padded form 400s there — and 502s on any core without that validation. Tests evaluate the routing expression the way n8n does, asserting what actually goes on the wire rather than the shape of the template, and pin the guard and the expression together so neither regresses alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The field is type:'string', but an expression resolves to whatever it
evaluates to — `={{ $json.id }}` over a numeric column yields a number.
The guard's `typeof raw === 'string'` check turned any non-string into
'', which returned early as "not provided" while the routing expression
still put the raw truthy value on the wire: the same bypass the trim fix
just closed, reached by a different route.
Non-strings now raise the same named error a source name does.
The wire expression coerces with String() rather than calling .trim()
directly, because routing values are evaluated BEFORE preSend — a bare
.trim() on a number throws a raw TypeError from inside the expression and
pre-empts the guard's readable message. Verified in a real n8n instance:
an expression yielding 123 now reports "Connection ID must be a
connection UUID, not "123"." instead of a TypeError.
Both findings were also raised independently by Entelligence on #23.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… one
Two defects in flattenHyperdoc, both reachable from Live → List Resources
the moment Simplify defaulted on for it in this branch.
Content loss. Not every hyperdoc node carries `text`. A HubSpot contact
arrives as {type:'person', name, email, company, children: []} — the
shape recorded from prod in the 2026-06-11 incident and already sitting
in this repo's own fixture. Walking only text/children flattened it to
'', and simplifyDocument then dropped the tree, so the email and company
were deleted outright. That is precisely the defect this file fixes for
Document → List, reintroduced on a different resource shape; the test
asserting it called the empty result "correct".
Leaf nodes now contribute their named scalar fields, labelled, so a model
reads "email: emailmaria@hubspot.com" rather than nothing. Only leaves: a
node with children holds its content there, and folding a document root's
own `title` into the text would duplicate a field the simplified row
already carries — the duplication this file's allow-list exists to stop.
The existing suite caught that, which is why the rule is narrow.
Cap. `remaining` counted node text but not the '\n' join, so a tree of
many small nodes returned close to 2x MAX_SIMPLIFIED_TEXT in separators
alone. Separators are now charged to the same budget. Measured end to end
against a real 266KB document: 2002 chars before, exactly 2000 now.
Both raised independently by Entelligence on #23.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| function matchedText(document: HyperspellDocument): string { | ||
| if (typeof document.summary === 'string' && document.summary.length > 0) { | ||
| return document.summary; | ||
| } | ||
| const highlights = Array.isArray(document.highlights) ? document.highlights : []; | ||
| return highlights | ||
| const fromHighlights = highlights | ||
| .map((highlight) => highlight?.text) | ||
| .filter((text): text is string => typeof text === 'string' && text.length > 0) | ||
| .join('\n\n'); | ||
| if (fromHighlights.length > 0) return fromHighlights; | ||
| return flattenHyperdoc(document.document, MAX_SIMPLIFIED_TEXT); |
There was a problem hiding this comment.
simplifyDocument returns summary or joined highlights without applying MAX_SIMPLIFIED_TEXT. Search's postReceive uses this helper for every query/answer hit, so a response with multiple 2,000-character highlights can still emit an unbounded row despite Simplify being advertised as the payload bound.
Prompt to fix with AI
Copy this prompt into your AI coding assistant to fix this issue.
Ensure matchedText applies MAX_SIMPLIFIED_TEXT to summary and concatenated highlights as well as the hyperdoc fallback. Add a regression test through simplifyDocuments using multiple long highlights or a long summary and assert the emitted text length is at most MAX_SIMPLIFIED_TEXT.
1 participant
What
simplifyDocument()builds each row'stextfromsummary/highlights, then drops the hyperdoc tree. Only the query path returnsScoredDocumentResponse(schemas.py:152) — the sole model carrying those fields./memories/list(documents.py:267),/memories/get/*and every/live/*route return the plainDocumentResponse(schemas.py:75): no summary, no highlights, body only in the tree.That produced two defects pointing in opposite directions.
1.
Document → Listdeletes its own content. Simplify is wired there with the default on, so since 0.7.0 every row comes back withtext: ""and the body dropped — metadata-only rows. Live in both 0.7.0 and 0.7.1 right now.2.
Document → Getand all three document-shapedLiveoperations had no bounding at all. They emit the full tree. The Live resource is the surface built for an AI-agent node (api/live.py:5), which made it simultaneously the most likely to feed a model and the only one shipping an unbounded payload. This is the token issue from the Aug 4 war room.search: search/answerdocument: listdocument: getlive: searchlive: get resourcelive: list resourcesHow
matchedText()falls back toflattenHyperdoc()— a depth-first walk of the tree'stextnodes, capped atMAX_SIMPLIFIED_TEXT = 2000to mirror the API's own per-highlight cap — whensummaryandhighlightsare both absent. The toggle is then wired to the four operations that lacked it, including a newsimplifyOnepostReceive because a get returns a bare object rather than a result array.Live → List Sourcesis deliberately excluded: capability descriptors, no tree to bound.Test changes, and why
Six existing tests failed against this and are updated on purpose:
this === undefined. That was safe only while those functions read no node parameters; they now read Simplify, and n8n always binds a real context. The stub was fixed, and two assertions whose actual subject was the envelope contract now passSimplify: falseexplicitly.text === ''for a document with neither summary nor highlights. That encoded the bug. It now asserts the tree fallback, and a separate new test covers the genuine empty case — no text anywhere in the tree.Verification
npm run build,npm run lint,npm test— 92/92 pass (was 82)Document → Listprobe:text: ""→ the real bodySimplify: falsereturns the raw tree untouched on every affected operationNot verified: none of this has run inside a real n8n instance. The suite exercises the built
dist/directly, same as the pre-existing tests. Whether this resolves what IntentHQ observes is unproven until 0.7.2 runs on their instance.Follow-ups
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.