Skip to content

fix(inspector): a component report names a snippet's file and lines only in its id - #6016

Merged
xuanruli merged 3 commits into
mainfrom
fix/inspector-slim-report
Oct 21, 2026
Merged

xuanruli merged 3 commits into
mainfrom
fix/inspector-slim-report

Conversation

@xuanruli

@xuanruli xuanruli commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

What

The component report (grep_components) spends less of its byte budget on fields the agent never acts on, so more source fits:

  • No fixed coverage sentences. Every report carried the same four disclaimers (566 bytes). The README already states those limits.
  • A snippet names its file and lines once, in its id (file:start-end). file, startLine and endLine repeated the id. targets is shown only when the query names more than one target; with one target it was always [0]. kind, code and uniqueInFile stay (uniqueInFile tells an agent whether the code can be used as an exact-match edit).
  • coTargets lists at most three elements, prioritizing elements outside all queried results, with coTargetsOmitted counting the rest. It was up to ten full element records.

Why

Measured in HyperFrames Desktop agent stress runs. An agent asked grep_components for .pt-key with maxBytes: 3000 and got 2,882 bytes with no snippets: eight coTargets (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 were omitted.

Same project, same query, before and after this change:

before after
maxBytes: 3000 2,882 bytes, 0 snippets 2,553 bytes, 1 snippet (the 378-byte line that creates the animation, kept first since #6003)
maxBytes: 12000 5,923 bytes, 6 snippets 4,136 bytes, the same 6 snippets

At the default budget the same answer is 30% smaller: snippet metadata 1,065 → 599 bytes, coTargets 1,221 → 457, coverage 566 → 0.

How

  • mapInspection: the response shows each kept snippet through shown(), which drops file/startLine/endLine (and targets for 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.
  • coTargets prioritizes unqueried elements before slicing to three; coTargetsOmitted appears only when some were left out.
  • README "Output" describes the new snippet shape and the coTargets cap.
  • Browser, film and live tests read a snippet's lines from its id through a small tests/snippetSource.ts helper.

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

  • New unit tests in tests/mapInspection.test.ts: no coverage and a one-target snippet of only id/kind/code/uniqueInFile; two targets keep targets; seven co-targets list three and count four. The first and third fail on main.
  • vitest run in packages/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.mjs and check-comment-citations.mjs pass.
  • Manual: the measurement above, Inspector from this branch and from main against the stress project.
  • Not covered: tests/film.ts and tests/live.ts (they need the film project and a live server); their change is the same id-based read as browser.ts.
  • Not covered: a live agent run with the new shape; that waits for a release.

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.

@jerrai-bot-heygen jerrai-bot-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 jerrai-bot-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@xuanruli
xuanruli enabled auto-merge October 21, 2026 08:43
@xuanruli
xuanruli added this pull request to the merge queue Oct 21, 2026
Merged via the queue into main with commit 1d422a2 Oct 21, 2026
98 of 99 checks passed
@xuanruli
xuanruli deleted the fix/inspector-slim-report branch October 21, 2026 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants