VOCION
Get a demo
← All posts

A query that only ever saw the last hour

The data-room collector shipped, ran after every sync, and filed nothing. One AND in a where-clause had pinned it to the sixty seconds around the run it followed — which made a documented constant unreachable and the dedupe code around it dead. vocion-core#463.

Vocion Teamvocion-v2.123.1
data-roomsconnectorssourcesretrievaloperationsschema

Status: shipped and observed working. vocion-core#463 (18e6ee31) fixes the collector that shipped in #449 (f5bb2cce).

A data room is the collection around one entity — a deal, a project, an engagement. Every transcript, thread and shared document filed to it with a weight, the rules the agent reads first, a dated timeline, the highlights. Documents are only as good as the room they are written from, so the room has to fill itself: after every source sync, a collector looks at what arrived, decides whether any of it clearly belongs to an open room, and files it with a match score. Anything merely plausible becomes a question for a person rather than a filing.

That is what the docs said. It is what the code was written to do. It had never filed anything.

Nobody noticed, which is the interesting part

The collector was not throwing. There was no error in a log, no failed run, no red anything. collectAfterSync ran after every sync, scanned its candidate set, found nothing that matched, and returned cleanly. Its receipt said so honestly: it had looked, and there was nothing. Rooms sat with the sources a person had filed by hand and no others, which looks exactly like "the collector is being appropriately conservative" right up until you check.

The tests passed too — and they were not bad tests. They handed planCollection a set of documents and asserted the right ones got filed, skipped or proposed. That logic was correct. The bug was one layer up, in the query that decides which documents planCollection ever gets to see, and a unit test that supplies its own input can never find it.

The where-clause

Here is the candidate query as it shipped, in packages/core/src/services/dataRooms/collector.ts:

const since = new Date(Date.now() - COLLECT_WINDOW_DAYS * 86_400_000);
const cutoff = new Date(payload.completedAt);
const touched = await db
  .select({ /* … */ })
  .from(knowledgeDocumentSchema)
  .where(and(
    eq(knowledgeDocumentSchema.orgId, orgId),
    eq(knowledgeDocumentSchema.sourceId, payload.sourceId),
    gte(knowledgeDocumentSchema.lastSeenAt, new Date(cutoff.getTime() - 60_000)),
    gte(knowledgeDocumentSchema.ingestedAt, since),
  ))
  .limit(COLLECT_MAX_DOCS * 4);

Both gte clauses are inside the same and. Read the third one: lastSeenAt >= cutoff - 60s, where cutoff is payload.completedAt — the completion time of the sync that just ran.

A document's lastSeenAt is updated when a connector sees it. So that clause says: only consider documents this connector touched in the last sixty seconds. On an install syncing hourly and incrementally, that is the handful of things that changed since the previous hour.

Which makes the fourth clause — the one everybody would point to if you asked how far back the collector looks — completely inert. Sixty seconds is inside thirty days for every document that has ever existed. The narrower condition wins, always, and the wider one is decoration.

Fifty-nine lines earlier in the same file:

/** How far back a run looks for material it has not filed yet. */
export const COLLECT_WINDOW_DAYS = 30;

That comment is a statement of intent, and it was false the day it was written. Not stale — never true. Everything in the file read as though thirty days were the window. The constant is exported, documented, named for exactly the behaviour a reader would expect, and the query underneath it could not reach past one minute.

The dead code downstream

There is a second tell, and it was sitting in plain sight the whole time.

planCollection carries real dedupe machinery. A filedIn map, built by roomsForDocuments, skips a document already filed to a room with reason: 'filed'. A dismissed set, built from each room's meta.unfiled array, skips a document a person has explicitly removed with reason: 'unfiled'. And a proposal upserts on the document's sourceRef so the same document never produces two questions.

All three of those exist for one reason: a run re-sees documents it has seen before. That is the only circumstance in which "have I already filed this?" is a question worth asking.

And a run never did. Each run saw a disjoint sixty-second slice; a document appeared in exactly one candidate set in its entire life. Three mechanisms, each correct, each tested, all of them unreachable.

This is the shape worth learning to recognise. When a chunk of defensive logic can never fire, something upstream is narrower than the design assumed — and the dead code is usually easier to spot than the where-clause, because it reads as an obviously sensible thing that nothing ever triggers.

The other bug, found by reading real data

While the query was open, someone looked at what emailsOf was actually returning against production rows. As shipped:

export function emailsOf(metadata: Record<string, unknown>): string[] {
  return [...new Set([str(metadata.host), str(metadata.from), ...strs(metadata.attendees), ...strs(metadata.to)]
    .filter((e): e is string => Boolean(e)))];
}

Straight passthrough of the header values. But a mail connector stores a From header the way mail stores it — Amy Larkin <amy@northwind.example> — and the caller takes the domain by splitting on @. So the strongest signal a room has, the counterparty's domain, arrived as:

northwind.example>

with a trailing angle bracket, and matched nothing. Ever. Domain matching is worth up to 0.8 in scoreRooms, and the automatic-filing threshold MATCH_HIGH is 0.75, so this one character was the difference between "files itself" and "never scores at all".

The fix is a small helper that reads the address out of the header rather than trusting it to be one:

function address(raw: string): string | null {
  const inAngles = /<([^<>]+)>/.exec(raw);
  const bare = (inAngles ? inAngles[1]! : raw).trim().replace(/^[\s"']+|[\s"',;]+$/g, '').toLowerCase();
  return bare.includes('@') ? bare : null;
}

It also splits multi-address strings on commas, strips quoting, lowercases, and drops anything without an @. Two bugs in the same code path, both of which produce silence rather than an error, and both of which a test with clean fixtures will never reproduce — because nobody writes 'Amy Larkin <amy@northwind.example>' into a fixture when 'amy@northwind.example' is what the function is obviously for.

The new candidate set

The window clause and the touched clause are now ored rather than anded, and "what names an open room" joins them:

.where(and(
  eq(knowledgeDocumentSchema.orgId, orgId),
  eq(knowledgeDocumentSchema.sourceId, payload.sourceId),
  gte(knowledgeDocumentSchema.ingestedAt, since),
  or(gte(knowledgeDocumentSchema.lastSeenAt, new Date(cutoff.getTime() - 60_000)), ...alsoInWindow),
))
.orderBy(desc(knowledgeDocumentSchema.lastSeenAt))
.limit(COLLECT_MAX_DOCS * 4);

alsoInWindow is built from roomSignalTerms(rooms) — a room's domain, client name, codename and aliases, matched against the document title and metadata text. On a CRM connector it is instead every deal, because a deal with no room names none and the whole point is to find the ones that should have a room.

Two design notes from the commit that are worth more than the diff:

roomSignalTerms selects on the room's identity and never on its cast. Half a cast is our own side, and "every message naming our partner" is the whole mailbox. Identity selects, people score.

And the ordering is not cosmetic. .orderBy(desc(lastSeenAt)) in front of a limit is what makes the candidate set deterministic; without it, a capped query returns whatever the planner felt like and the collector's behaviour becomes a function of table statistics.

Verification, which is the part worth telling

A unit test that supplies its own rows could not have found this bug, so a unit test alone was never going to be sufficient evidence that it was fixed.

There is a regression test — collector.window.test.ts, whose header comment pins the defect in its own words ("the candidate query used to require last_seen_at >= the run's cutoff, so a run only ever saw the documents that connector had just changed"), asserting that a document ingested and last seen seven days ago, naming an open room, is filed even though the current sync touched something unrelated; and that material naming no room, and material past the thirty-day window, are both left unscanned.

But before the merge, the fixed query was replayed over 991 real documents on the deployment we operate — read-only, against the production database, with the scoring run but nothing written. That is the run that turns "the test passes" into "this would have filed these, and these are right".

Then it went out. On the very next sync after the deploy, the collector filed seven sources onto one live room, scoring between 80% and 100%. Not a fixture. Not a replay. Seven transcripts and threads that had been sitting unfiled in the index, that the room should have had from the beginning.

A filed source reads as one row in the room's Sources list: the document's own title, the channel and date it came from, filed automatically where a hand-filed source names the person, the match score as a percentage, and the evidence phrase that earned it — on most of these seven, the counterparty's email domain, which is the signal that had been arriving with a > stuck to it until that morning. Each row carries a Remove control, which unfiles it and writes the document's id into the room's meta.unfiled so the collector does not propose it again. That control is the reason auto-filing is allowed to be automatic at all: the undo is one click and the collector remembers it.

The second sync after the deploy filed nothing new, which is the other half of the evidence. A collector that had over-corrected into filing everything it could reach would have kept going. It saw the same documents again — which it had never once done before — recognised them through the filedIn map, and skipped them.

Two lessons

A constant that documents an intention is not a query that implements it. COLLECT_WINDOW_DAYS = 30, with a comment saying exactly what it is for, sat above a query that could not honour it. Every reader of that file — including the people who wrote the docs page describing a thirty-day window — took the constant as the behaviour. The name and the comment were load-bearing documentation for a mechanism that did not exist. If a constant expresses a policy, something should assert that the policy holds; otherwise the constant is a comment with a type.

A feature nobody has ever seen work is not shipped. The collector had a pull request, tests, a docs page and a release post. What it did not have was one observed instance of the thing it exists to do. The gap between "merged" and "does its job in production" is where this bug lived for the whole life of the feature, invisible, because everything about it looked finished. The build root's own definition of done says this out loud: a feature is done when it is live, verified by a probe a person could repeat. Seven sources on a room is that probe. The pull request was not.

Which suggests the check worth building next, and it is not a test: a measure that notices a collector which has run a hundred times and filed zero things. Silence that is indistinguishable from correctness needs a counter, not an assertion.

Where this lives in the repo

vocion-core#463 (18e6ee31).

The 991-document replay and the post-deploy filing were run on the deployment we operate and are reported here as measurements from that box; the numbers are not reproducible from the repository, and nothing in the repo asserts them.

Related: Data rooms and proposals, as plugins and A sync that tells you what it did.