Skip to content

Commit 021f49f

Browse files
ADFA-5094: fix subtree truncation correctness (review follow-up)
Two size-under-count bugs in buildLocalSubtree, both only reachable when a subtree exceeds the node cap: - The files subquery had an ORDER-BY-less LIMIT, so under truncation it could select a different node subset than the rows query and leave a built node without its files. Drop the LIMIT: the subquery now covers every kept node; files for nodes past the cap are just never looked up. - Truncation marked only the root incomplete, so deeper truncated folders under-counted silently. Mark every node whose range extends past the cutoff (rght > last kept lft) as incomplete, so the mapper reports unknown size instead of under-counting. Over-marking is safe; under-marking was the bug. Subtrees within the cap are unaffected: no truncation, complete children, exact sizes.
1 parent baacf0e commit 021f49f

1 file changed

Lines changed: 17 additions & 6 deletions

File tree

‎static/dashboard/sockets/kolibri.query.ts‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -297,15 +297,19 @@ export function buildLocalSubtree(nodeId: string): StudioNode | null {
297297

298298
// Real content files only: thumbnails and supplementary files are excluded so the
299299
// sizes match a normal import, the same exclusion StudioCatalogMapper makes.
300+
// No LIMIT on the subquery (unlike the rows query): it must cover every node the rows
301+
// query kept — an ORDER-BY-less LIMIT here could select a different subset and leave a
302+
// built node without its files, under-counting its size. Files for nodes past the cap
303+
// are simply never looked up.
300304
const files = db.prepare(
301305
`SELECT f.contentnode_id AS node_id, f.preset AS preset, lf.file_size AS file_size
302306
FROM content_file f
303307
JOIN content_localfile lf ON lf.id = f.local_file_id
304308
WHERE f.thumbnail = 0 AND f.supplementary = 0
305309
AND f.contentnode_id IN (
306310
SELECT id FROM content_contentnode
307-
WHERE channel_id = ? AND lft >= ? AND rght <= ? LIMIT ?)`,
308-
).all(root.channel_id, root.lft, root.rght, SUBTREE_NODE_CAP) as FileRow[];
311+
WHERE channel_id = ? AND lft >= ? AND rght <= ?)`,
312+
).all(root.channel_id, root.lft, root.rght) as FileRow[];
309313

310314
const filesByNode = new Map<string, StudioFile[]>();
311315
for (const f of files) {
@@ -333,10 +337,17 @@ export function buildLocalSubtree(nodeId: string): StudioNode | null {
333337
}
334338

335339
const rootNode = byId.get(root.id) ?? null;
336-
// Hitting the cap means the deepest nodes were dropped; mark the root incomplete so
337-
// the mapper reports its subtree size as unknown rather than silently under-counting.
338-
if (rootNode && rows.length >= SUBTREE_NODE_CAP) {
339-
rootNode.children.more = { cursor: 'truncated' };
340+
// Truncation correctness. Rows are ordered by lft and capped, so the dropped nodes are
341+
// exactly those with lft beyond the last one kept. Any node whose range (lft..rght)
342+
// extends past that cutoff therefore has a missing descendant, so mark it incomplete —
343+
// the mapper then reports its subtree size as unknown instead of silently under-counting.
344+
// Over-marking is safe; under-marking is the bug. Nodes fully within the cutoff keep a
345+
// complete child set and an exact size.
346+
if (rows.length >= SUBTREE_NODE_CAP) {
347+
const cutoffLft = rows[rows.length - 1].lft;
348+
for (const node of byId.values()) {
349+
if (node.rght > cutoffLft) node.children.more = { cursor: 'truncated' };
350+
}
340351
}
341352
return rootNode;
342353
} finally {

0 commit comments

Comments
 (0)