Repository navigation
fix(inspector): a component report names a snippet's file and lines only in its id - #6016
Conversation
jerrai-bot-heygen
left a comment
There was a problem hiding this comment.
Reviewed exact 740163d80319d415f5e0e929f7b6d71a983beeb0. The compact shape removes redundant file/line fields while preserving snippet IDs, and the updated reader in internal #4997 accepts both old and new forms; that reader should land before the new inspector package is adopted. Budget/paging still serializes the reduced response before counting it, with nextOffset and omitted entries; hosted Lint/Typecheck/support tests executed green, while several other jobs remained in progress at check. One source-level regression is inline: the new per-result three-co-target cap can hide the sole unqueried element affected by a shared tween from every result when four other targets are queried. This is not covered by the single-target cap test. I did not run a live browser stress trace or merge. — Jerrai
| .map((element) => [element.ref, element]), | ||
| ).values(), | ||
| ].slice(0, 10), | ||
| coTargets: coTargets.slice(0, CO_TARGETS_SHOWN), |
There was a problem hiding this comment.
A fixed first-three cap can hide the only unqueried side effect of a shared tween. Query four elements A–D when one animation targets A, B, C, D, then X in that order. Each result removes only its own element before slice(0,3), so all four reports display other queried elements and none names X; each merely reports one omitted co-target, with no pagination path for that identity. Previously the ten-entry list included X, and this loses useful side-effect context even with ample maxBytes. Please prioritize distinct unqueried co-targets across the queried results (or expose a way to retrieve the omitted identities), and add the four-query/fifth-target regression.
There was a problem hiding this comment.
Fixed in acd38cc: rank co-targets outside all queried result elements first, then apply the existing three-item cap. Added the four-query/fifth-target regression; it fails before the fix and all 10 mapping tests pass afterward. Normal lint, format, typecheck, fallow and commit hooks pass.
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed full compact-report delta and successor acd38ccf50073114aef6a87b185ea26a95babf7c. No additional blocker. Jerrai's four-query/fifth-target finding is repaired: the unqueried reference ranks first before the three-item cap. My exact-source control passes this head and fails 740163d8.
packages/inspector/src/mapInspection.ts:18–24,130–150 compacts only returned snippets; the catalog retains full source fields for selection, explicit IDs and position sorting. Independent controls cover full catalog retention, single/multiple-target shape, deduplicated co-target count, all-fit reversed explicit IDs and127 UTF-8 budget boundaries (60 advancing continuations; no duplicate returns). The base fails the compact-shape control.
I inspected linked HFI #4997 source 7eda2ca057c703ce202a4e7772c6fac9c366c982 without its PR APIs: its actual output schema accepts producer compact output and legacy shape; both file-location readers accept both, exact bytes are required for source observation, and traversal/mismatched text are rejected. The predecessor schema rejects compact output, so the documented reader-first adoption order is real. This is source compatibility proof, not a claim that #4997 landed or that a package was shipped.
Current-head Lint ran green (119124172156, underlying log inspected), and the newer capture check119124119582 succeeds after an older cancelled check; its log says captures present or nothing to show. New-head support/Typecheck/runtime jobs were still running at08:39Z. Earlier740-head support executed51 inspector tests/9files, but that does not substitute for successor CI. Browser/stress measurements remain author evidence; no reviewer browser/model/eval or merge.
— Magi / desktop-review
Verdict: APPROVE
Reasoning: The compact/catalog contract and repaired co-target priority pass discriminating controls; remaining hosted checks and reader-first package adoption retain their normal gates.
jerrai-bot-heygen
left a comment
There was a problem hiding this comment.
Re-reviewed exact acd38ccf50073114aef6a87b185ea26a95babf7c, superseding my co-target changes request. The report now builds queriedRefs from all query results and sorts each tween’s de-duplicated co-targets so unqueried elements come first before the unchanged three-item cap and omitted count. The new four-query/fifth-target test asserts that the previously hidden X appears first in every result. The compact ID/canonical catalog and serialized-budget/paging paths remain unchanged, and internal #4997’s dual-shape reader already accepts both old and compact responses; adopt/land that reader before this producer version reaches Desktop. At click time Lint and Typecheck executed green; support/Windows/runtime CI still had pending jobs, not counted as passed. Browser/stress numbers remain author evidence. No live model call or merge by me. — Jerrai
What
The component report (
grep_components) spends less of its byte budget on fields the agent never acts on, so more source fits:coveragesentences. Every report carried the same four disclaimers (566 bytes). The README already states those limits.id(file:start-end).file,startLineandendLinerepeated the id.targetsis shown only when the query names more than one target; with one target it was always[0].kind,codeanduniqueInFilestay (uniqueInFiletells an agent whether the code can be used as an exact-match edit).coTargetslists at most three elements, prioritizing elements outside all queried results, withcoTargetsOmittedcounting the rest. It was up to ten full element records.Why
Measured in HyperFrames Desktop agent stress runs. An agent asked
grep_componentsfor.pt-keywithmaxBytes: 3000and got 2,882 bytes with no snippets: eightcoTargets(1,221 bytes), the coverage sentences (566) and the rest of the metadata left no room under the budget for even a 61-byte snippet, so all six wereomitted.Same project, same query, before and after this change:
maxBytes: 3000maxBytes: 12000At the default budget the same answer is 30% smaller: snippet metadata 1,065 → 599 bytes,
coTargets1,221 → 457,coverage566 → 0.How
mapInspection: the response shows each kept snippet throughshown(), which dropsfile/startLine/endLine(andtargetsfor a one-target query). The catalog keeps full snippets, so paging, explicit-ID requests and file-order sorting are unchanged. The budget loop measures the shown form.coTargetsprioritizes unqueried elements before slicing to three;coTargetsOmittedappears only when some were left out.coTargetscap.tests/snippetSource.tshelper.The response shape changes. The Desktop agent's output schema and the two readers of a snippet's file accept both shapes in heygen-com/hyperframes-internal#4997, which lands before this ships there.
Test plan
tests/mapInspection.test.ts: nocoverageand a one-target snippet of onlyid/kind/code/uniqueInFile; two targets keeptargets; seven co-targets list three and count four. The first and third fail on main.vitest runinpackages/inspector: 51 passed.bun tests/browser.ts <gsap.min.js>: passes, including "literal snippets equal original bytes" read through the id.tsc --noEmit,oxlint,oxfmt --check, pre-commit hooks,comment-ratchet.mjsandcheck-comment-citations.mjspass.Inspectorfrom this branch and from main against the stress project.tests/film.tsandtests/live.ts(they need the film project and a live server); their change is the same id-based read asbrowser.ts.Review follow-up: the four-query/fifth-target regression reproduced before the fix and now passes. All 10 focused mapping tests and normal pre-commit/commit-msg hooks pass locally at
acd38ccf50073114aef6a87b185ea26a95babf7c. The browser/stress measurements above remain author-reported evidence from the initial head.