Skip to content

Commit 03ea949

Browse files
committed
fix(tables,knowledge): spare a live window, and restore the truncation notice
A lease needs its heartbeat interval to sit well under its TTL. The dispatch heartbeat is stamped between windows, not during them, and `batchTriggerAndWait` checkpoints the loop for the whole window — so the interval is really "one window", which nothing bounds: the window ends when its cells do, and the in-process path has no ceiling at all. A window outliving the stale threshold had its dispatch cancelled while it was plainly alive. Its cells carry the signal the checkpointed parent cannot — `updatedAt` on every in-flight row execution, written by the cell tasks themselves. Both signals must be stale before a dispatch is reclaimed, so a slow window is spared for as long as its cells keep reporting while a run with nothing beating and nothing executing is still collected. The subquery rides the partial `(table_id, status)` index that already covers exactly those three statuses. Bounding the workbook conversion also made its truncation notice unreachable: the converted length can no longer exceed the window it was compared against, so every sheet larger than the preview cap silently stopped reporting that it had been cut. Compared against the declared row count instead, which is what the comparison meant before the conversion was bounded.
1 parent 5b25771 commit 03ea949

4 files changed

Lines changed: 62 additions & 3 deletions

File tree

apps/sim/lib/file-parsers/xlsx-parser.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,15 @@ export class XlsxParser implements FileParser {
176176
contentSize += chunkContent.length
177177
}
178178

179-
if (actualRowCount > rowsToProcess) {
179+
/**
180+
* Compared against the DECLARED row count, not the converted one. The
181+
* conversion is now bounded to the preview window, so the converted
182+
* length can never exceed it — comparing the two made this unreachable
183+
* and silently dropped the notice from every sheet larger than the cap.
184+
*/
185+
if (rowCount > rowsToProcess) {
180186
content += truncationNotice(
181-
`${actualRowCount.toLocaleString()} total rows, showing first ${rowsToProcess.toLocaleString()}`
187+
`${rowCount.toLocaleString()} total rows, showing first ${rowsToProcess.toLocaleString()}`
182188
)
183189
truncated = true
184190
}

apps/sim/lib/file-parsers/xlsx-preview-bound.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,4 +67,16 @@ describe('XlsxParser preview bound', () => {
6767
expect(result.content).toContain('header-a')
6868
expect(result.content).toContain('row-2-b')
6969
})
70+
71+
it('still reports truncation for a sheet larger than the preview window', async () => {
72+
const result = await new XlsxParser().parseBuffer(inflatedRangeWorkbook())
73+
74+
/**
75+
* Bounding the conversion made the converted length equal the window, so
76+
* comparing it against the window could never be true — the notice silently
77+
* disappeared from exactly the large sheets it exists for.
78+
*/
79+
expect(result.metadata?.truncated).toBe(true)
80+
expect(result.content).toContain('200,000 total rows')
81+
})
7082
})

apps/sim/lib/table/dispatcher.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -826,10 +826,35 @@ export async function cancelStaleDispatches(
826826
staleBefore: Date,
827827
limit: number
828828
): Promise<DispatchRow[]> {
829+
/**
830+
* Dead means nothing about the dispatch has moved — neither the loop nor the
831+
* work it is waiting on.
832+
*
833+
* The dispatch's own heartbeat is stamped between windows, not during them,
834+
* and `batchTriggerAndWait` checkpoints the loop for the whole window, so a
835+
* long window leaves the heartbeat untouched while the dispatch is plainly
836+
* alive. A lease needs its heartbeat interval to sit well under its TTL; this
837+
* one cannot promise that, because the window is bounded by the cells' own
838+
* timeouts and the in-process path has no ceiling at all.
839+
*
840+
* Its cells carry the signal the checkpointed parent cannot: `updatedAt` on
841+
* every in-flight row execution, written by the cell tasks themselves. Both
842+
* must be stale before a dispatch is reclaimed, so a slow window is spared for
843+
* as long as its cells keep reporting, and a genuinely dead run — nothing
844+
* beating, nothing executing — is still collected. The subquery rides the
845+
* partial `(table_id, status)` index that already covers exactly these three
846+
* statuses.
847+
*/
829848
const isStale = () =>
830849
and(
831850
inArray(tableRunDispatches.status, [...ACTIVE_DISPATCH_STATUSES]),
832-
sql`COALESCE(${tableRunDispatches.heartbeatAt}, ${tableRunDispatches.requestedAt}) < ${sql.param(staleBefore, tableRunDispatches.heartbeatAt)}`
851+
sql`COALESCE(${tableRunDispatches.heartbeatAt}, ${tableRunDispatches.requestedAt}) < ${sql.param(staleBefore, tableRunDispatches.heartbeatAt)}`,
852+
sql`NOT EXISTS (
853+
SELECT 1 FROM ${tableRowExecutions}
854+
WHERE ${tableRowExecutions.tableId} = ${tableRunDispatches.tableId}
855+
AND ${tableRowExecutions.status} IN ('queued', 'running', 'pending')
856+
AND ${tableRowExecutions.updatedAt} >= ${sql.param(staleBefore, tableRowExecutions.updatedAt)}
857+
)`
833858
)
834859

835860
// Claimed as explicit ids first, then updated by id, so the bound is evaluated

apps/sim/lib/table/stale-dispatch-recovery.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,22 @@ describe('cancelStaleDispatches', () => {
100100
expect(chunks).toContain('tableRunDispatches.requestedAt')
101101
})
102102

103+
it('spares a dispatch whose cells are still reporting', async () => {
104+
await cancelStaleDispatches(STALE_BEFORE, 200)
105+
106+
const chunks = collectChunks(dbChainMockFns.where.mock.calls[0][0])
107+
/**
108+
* The dispatch's own heartbeat is stamped between windows, not during them,
109+
* and the loop is checkpointed for the whole window — so a long window
110+
* leaves it untouched while the dispatch is plainly alive. Its cells carry
111+
* the signal the checkpointed parent cannot, and both have to be stale
112+
* before the row is reclaimed.
113+
*/
114+
expect(chunks.some((chunk) => chunk.includes('NOT EXISTS'))).toBe(true)
115+
expect(chunks).toContain('tableRowExecutions.updatedAt')
116+
expect(chunks).toContain('tableRowExecutions.tableId')
117+
})
118+
103119
it('emits the terminal event so a stuck client overlay clears', async () => {
104120
await cancelStaleDispatches(STALE_BEFORE, 200)
105121

0 commit comments

Comments
 (0)