Skip to content

Commit a69cc22

Browse files
Krandeclaude
andcommitted
fix(viewer): the Clickable surfaces toggle never reached the request
Ticking it did nothing: the reconvert ran and reported success, and the GLB came back with no face regions. My own bug, from the commit that added the toggle. reconvert() reads faceRegionsOn but the useCallback dep array listed only [current, reconverting, serializerSel]. So the memoized callback kept the closure from an earlier render, where faceRegionsOn was false, and sent extraOptions: undefined however the checkbox was set. Confirmed against the deployment before fixing: audit 88603's reconvert GLB carries id_hierarchy + draw_ranges_node0 and no face_ranges, while POSTing the same job to the same API with conversion_options {"face_regions": true} produces face_ranges_node0. The backend chain — allowlist (face_regions is in ConverterRegistry.all_options), worker _env_map_full, ADA_STREAM_TESS_FACE_REGIONS, native_step_to_glb — was doing its job the whole way. react-hooks/exhaustive-deps is exactly the rule that catches this, and it never ran: eslint isn't configured for this package (v9 wants a flat config; `npx eslint` only prints the migration guide), so every dep array here is hand-maintained. Worth wiring up separately — this class of bug is silent by construction, and no test in this suite can see it (the frontend tests are node --test over pure modules, with no React renderer). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 7de0936 commit a69cc22

1 file changed

Lines changed: 6 additions & 1 deletion

File tree

src/frontend/src/components/viewer/GalleryControls.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,12 @@ const GalleryControls: React.FC = () => {
266266
} finally {
267267
setReconverting(false);
268268
}
269-
}, [current, reconverting, serializerSel]);
269+
// faceRegionsOn belongs here: without it useCallback hands back a closure that captured the
270+
// value from an earlier render, so ticking the toggle re-renders but the memoized reconvert
271+
// still sends the old answer — the request goes out without face_regions and the conversion
272+
// reports success. react-hooks/exhaustive-deps would have caught it; eslint isn't wired up
273+
// for this package, so these arrays are hand-maintained.
274+
}, [current, reconverting, serializerSel, faceRegionsOn]);
270275

271276
// NOTE: every hook must be declared before the `if (!enabled) return null` early return below —
272277
// React counts hooks per render, so a hook after the return changes the count when the gallery is

0 commit comments

Comments
 (0)