Skip to content

feat(cli): check-review inspect attaches the inspector's report to each framed finding - #6409

Merged
xuanruli merged 13 commits into
mainfrom
feat/check-review-inspect
Oct 26, 2026
Merged

xuanruli merged 13 commits into
mainfrom
feat/check-review-inspect

Conversation

@xuanruli

@xuanruli xuanruli commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

What

A third check-review field in HF_TOOL_CONTEXT: inspect (boolean, default false).

When inspect and findingFrames are both on, the session that takes the finding frames also runs the component inspector (@hyperframes/inspector) on each framed finding while its frame is on screen. The inspector's report is added to that finding, in the inspector's own format, as a new inspect field.

  • findingFrames off: inspect does nothing.
  • Without the capability, output is unchanged.

Why

A framed finding says what is wrong and shows the frame, but not where it comes from in the source. To fix it, an agent has to find the element's markup, the CSS that matches it and the GSAP calls that move it. The inspector already answers that from the live page. Running it in the frames session costs almost nothing, so the review gets the source context without another command.

Related work

Based on main. #6401 (findingFrames) merged into main as 12ae82aa8ab5cef317dc28a6bfbf6913dc188ca4 at 2026-10-26 08:45:29 UTC; #6400 is also merged. Landed main was integrated with signed merge d06422ef24ae8ef0fe19a7b983ac8529c17554e2. GitHub automatically retargeted this child to main when the parent landed; the diff is only inspect and its focused repairs.

How

  • Parsing. check-review.inspect is parsed in toolContext.ts the same way as the other two fields, and read through the same checkReview() helper in commands/check.ts. The option is set only when the field is true.
  • Targets. For each frame time, the pipeline lists the distinct targets of the findings framed at that time: { hfId } when the finding has dataAttributes["data-hf-id"], else { selector } (findingFrameRequests in checkFindingFrames.ts). With inspect off, the requests are exactly as before.
  • Page. When a request has targets, prepareFindingInspector (new, checkFindingInspect.ts) bundles the project with scriptProvenance and onRead, and calls the inspector's embedding API, prepareLiveInspection from @hyperframes/inspector/live. That API puts the GSAP hooks first in <head>. The frames session serves that HTML instead of the plain check page. Once the page is ready, the inspector is given the HTML the server actually answers (finalizeDocument). That HTML is read through server.answer when the sandbox refused listen(), else fetched.
  • Inspecting. For each time: seek, screenshot, then one inspect call per target with { targets: [target], descendants: false } under the inspector's default byte budget. The live API refuses time (live_query_does_not_seek), because the caller seeks. Check has already seeked the page to frameTime, so the report is taken at that time. The report has no visualState; the finding's own frameTime already gives the time. Nothing is added to the report: no bbox at frame, no progress, no ancestor walk.
  • Failure. A selector matching multiple elements yields an ambiguous_selector error rather than attaching mixed source snippets; the ambiguity count includes candidates omitted by the inspector byte budget. One failed inspection makes that finding inspect: { "error": "<message>" }, and the other findings keep going. If preparation fails, the frames are still captured, and every targeted finding gets the { error }. Neither ever fails the check.
  • Deadline. Cancellation is checked after inspector preparation, readiness and each inspection, so a stopped operation cannot continue into later targets or frames. Each inspect call enters the frames phase, the same way each frame does. Inspection runs inside the frames deadline.run, so a timeout or stop during inspection is handled like a frames timeout: a runtime failure, and no frame or inspect fields.
  • Packaging. @hyperframes/inspector is a workspace devDependency and bundled into the CLI (noExternal), like core and producer. The CLI is published with npm publish, which would not rewrite a workspace:* runtime dependency. The inspector reads its page runtime from beside its own module, so build-copy.mjs copies the required inspector/dist/inspection.js to cli/dist/inspection.js; a missing runtime fails the build. The module is imported dynamically, so it loads only when something is inspected.
  • captureFindingFrames now returns { written, inspected } instead of string[].
  • The unused @types/mime-types devDependency is dropped. The dependency audit flags it once packages/cli/package.json changes.

Example inspect value (trimmed), from a container_overflow finding on #cz-w1 > span:nth-of-type(1) (data-hf-id="hf-1s9k", frameTime 17.527):

{
  "generation": "79914b17-…",
  "sourceVersion": "05e707db…",
  "reused": true,
  "results": [
    {
      "target": 0,
      "status": "found",
      "candidates": [
        { "ref": "hfi:79914b17-…:25", "tag": "span", "id": "", "hfId": "hf-1s9k", "classes": ["cz-wi"], "composition": "cta", "withinId": "cz-w1" }
      ],
      "animations": 4,
      "coTargets": [],
      "unmappedAnimations": 0,
      "scopeTruncated": false
    }
  ],
  "snippets": [
    { "id": "compositions/cta.html:49-49", "kind": ["matched CSS candidate"], "code": "  .cz-wi { display: block; font: 800 158px/190px \"Inter\", sans-serif; … white-space: nowrap; }\n", "uniqueInFile": true },
    { "id": "compositions/cta.html:215-215", "kind": ["html opening tag"], "code": "  <div data-hf-id=\"hf-jpgt\" id=\"cz-line\"><div … id=\"cz-w1\" class=\"cz-w\"><span data-hf-id=\"hf-1s9k\" class=\"cz-wi\">Roll</span></div>…\n", "uniqueInFile": true },
    { "id": "compositions/cta.html:255-258", "kind": ["animation creation", "animation source dependency"], "code": "function steps(target, times, props) {\n  tl.set(target, pick(props, 0), 0);\n  …\n}\n", "uniqueInFile": true },
    { "id": "compositions/cta.html:468-468", "kind": ["animation caller"], "code": "steps('#cz-w1 .cz-wi', [5.25, 5.333, 5.417, 5.5], { yPercent: [110, 70, 30, 0] });\n", "uniqueInFile": true },
    "… 6 more: composition roots and host, animation source dependencies"
  ],
  "omitted": [],
  "omittedCount": 0,
  "nextOffset": null,
  "recordLimitReached": false,
  "hooked": true,
  "debugAvailable": true,
  "elapsedMs": 9
}

Test plan

  • Unit tests added/updated
    • toolContext.test.ts: inspect parses as a boolean, defaults to false, and is off for a non-true value.
    • commands/check.test.ts:
      • the CLI sets inspect only from the capability;
      • an hfId target when data-hf-id is present, else the selector;
      • each framed finding gets the capture's report, and one failed inspection gets { error } while the others keep theirs and the check does not fail;
      • with inspect alone, or findingFrames alone, no targets are asked for and no inspect field is added (the inspect-alone report is byte-identical to the plain one).
    • checkBrowser.test.ts:
      • the frames session serves the inspector's HTML and hands it the server;
      • it inspects each target right after its frame's seek and screenshot, in order;
      • a failed target does not stop the rest;
      • the inspector is not prepared when no request asks for it.
    • checkFindingInspect.test.ts:
      • bundling uses provenance, and finalizing uses the server's own answer;
      • a single-target, no-descendants request;
      • one throw becomes { error };
      • a preparation failure, or a page that cannot be read back, becomes every target's { error }.
    • checkFindingInspect.chromium.test.ts (runs when PUPPETEER_EXECUTABLE_PATH is set, like checkBrowser.chromium.test.ts): a real project with real GSAP, on a real page through the real static server. By hfId and by selector, it returns found with 1 animation and snippets of kind html opening tag, animation creation and matched CSS candidate; a missing selector returns missing.
    • Run: vitest run on check.test.ts, toolContext.test.ts, checkBrowser.test.ts, checkFindingInspect.test.ts, checkFindingInspect.chromium.test.ts, checkBrowser.chromium.test.ts, bundleWithLocalizedFonts.test.ts: 228 passed. The non-Chromium ones also pass with packages/inspector/dist removed, as in the Test (cli) job. tsc --noEmit in packages/cli passes, and so do oxlint and oxfmt --check on the touched files, the pre-commit fallow audit, and bun install --frozen-lockfile.
  • Manual testing performed
    • Wall clock of check --json with findingFrames on, on four real projects, inspect off and on. Three interleaved runs each, median, built CLI, isolated HYPERFRAMES_HISTORY_ROOT:

      Project Framed findings inspect off (s) inspect on (s) Delta (s)
      instagram_reel 1 7.03 7.04 +0.01
      hyperframe_update_highlights 1 5.51 5.51 0.00
      google_playground_launch_film 0 4.36 4.44 +0.08 (nothing to inspect; noise)
      bench_m1_dsh_sonnet 33 7.21 7.34 +0.13
    • Coverage: 35 of 35 framed findings got an inspect report with status found, with no { error }. All 33 on bench_m1 were targeted by hfId: 26 contrast, 7 layout.

    • Byte-identical check against the base branch build (5e14d8bd9), on instagram_reel and bench_m1. These configurations gave the same output except the per-run UUIDs in the report and snapshot paths:

      • no HF_TOOL_CONTEXT;
      • findingFrames only;
      • inspect only.
  • Documentation updated: docs/developers/harness-tool-context.mdx, check-review section, with the inspect field and the finding's inspect field.
  • Comments follow CONTRIBUTING.md "Comments"

Independent followthrough validation (a003c3681)

  • Substantive caller/contract/lifecycle self-review found continued inspection after cancellation. Stop and timeout regressions failed before the fix and pass after; prepare/ready cancellation also prevents later work and checks cleanup. Removed two redundant doc comments to repair the actual Comments gate.
  • 241 tests in eight focused CLI files passed, including both headless Chromium suites. Initial workspace build, rebuilt CLI, CLI typecheck, lint/format, comment citation/ratchet and lazy-dependency checks passed using existing dependencies.
  • Copied bin, dist and package.json outside the checkout: top-level --help and --version work with no dependencies. check --help needs the declared hono dependency. With only declared runtime dependency links and no workspace package links, the built CLI checked a local GSAP overflow fixture in plain, inspect-only, frames-only and frames+inspect modes. Both framed findings returned native found reports with markup/CSS/animation snippets; no visualState. Plain/inspect-only differed only in per-run saved IDs/paths. Copied inspection.js matches the inspector build.
  • The four-project timing measurements above remain author-reported. No full CI/coverage, live-model, UI/Electron, Mintlify, deployment or publication was run in this followthrough.

Review repairs (6ad5162a1)

  • Genesis identified ambiguous fallback selectors and an optional runtime copy. The headless nested-DOM regression failed before and passes after returning the existing { error } form for multi-candidate selector reports; unique targets retain the native report. Unit coverage includes candidates omitted by the byte budget.
  • The runtime copy is now required. With only this worktree's generated inspection.js temporarily absent, the previous script exited 0 and the repaired script exited 1. The artifact was restored and build-copy/full CLI build passed.
  • 218 tests in four focused suites passed, then all 5 inspector unit tests passed after adding omitted-candidate coverage. Typecheck, lint/format and comment checks passed.

Main integration (d06422ef2)

  • Resolved ten squash-history conflicts after verifying each main-side conflicted file exactly matched the parent already incorporated in this branch. Preserved the reviewed child content; other landed main changes were retained. The diff against main is the intended 18 files.
  • All 243 tests in the eight focused CLI files passed after integration, including headless Chromium. CLI build, hook typecheck/lint/format, comment citation/ratchet and lazy-dependency checks passed.

Windows prerequisite repair (0e17b6bdd)

  • Main-target Windows render exposed a selective-build gap: the job omitted the inspector build, so the required inspection.js copy failed. Added inspector immediately before CLI in the Windows render build list.
  • Rebuilt inspector from absent generated output and then built CLI locally; both passed. YAML parsing/order and formatting checks passed. The new hosted Render on windows-latest job also passed. The final diff adds this workflow prerequisite to the 18 application/docs/test/build files above.

Server parser repair (ba4e33413)

  • CodeQL traced prepared inspection HTML into the viewport helper's ambient DOMParser. The helper now directly uses the existing LinkeDOM server parser, matching neighboring HTML helpers and avoiding dependence on an ambient browser parser. No alert suppression or filter was added.
  • A regression with a throwing ambient parser and script/event HTML fails before the change and passes after, preserving viewport dimensions without invoking the ambient parser or executing the payload. All 290 tests in six focused suites passed, including headless Chromium; CLI build, typecheck, lint and formatting passed.
  • Hosted CodeQL analysis and Windows render pass on this head. This adds only the viewport helper and its regression to the previously described diff.

Not covered

  • The inspector sees GSAP animations only: not CSS animations or transitions, Web Animations (WAAPI) or canvas drawing. A finding moved by those still gets its markup and CSS snippets, but animations counts only GSAP.
  • inspect is only on findings that got a frame: error and warning findings in layout and contrast with a real bbox, at most 60 distinct times.
  • When the deadline fires during frames, no frames or inspections are kept. That is the same as a frames timeout today; there is no partial result.
  • The report is the inspector's default 12,000-byte budget. Larger reports list the rest in omitted / nextOffset, which check does not follow.

The --skip-contrast-in-motion flag and HYPERFRAMES_CHECK_SKIP_CONTRAST_IN_MOTION
env give way to a check-review capability in HF_TOOL_CONTEXT, so only a harness
turns it on. The skill text and manifest go back to main.
…ng time

With HF_TOOL_CONTEXT check-review.findingFrames, every error/warning finding in
layout or contrast with a real bbox gets frameTime (the middle sample of its
longest held run, by the heldMs gap rule) and frame (the absolute path of an
unannotated full frame shared by every finding at that time). One seek and
screenshot per distinct time, capped at 60 times with errors first, into the
run's snapshot folder; not listed in snapshots, history or events. Without the
capability, output is unchanged.
…ch framed finding

With findingFrames on, the finding-frames page is served with the
inspector's GSAP hooks. After each frame is captured, every finding framed
at that time is inspected by its data-hf-id, else its selector, and the
inspector's own report is attached as the finding's inspect field. A failed
inspection becomes that finding's { error } and never fails the check.
Without the capability nothing changes.

Drops the unused @types/mime-types devDependency that the dependency audit
flags once packages/cli/package.json is touched.

@heygengenesis heygengenesis Bot 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.

Verdict: APPROVE · 0 blocker · 1 important · 1 nit · head a003c36

Capability gating, the seek → screenshot → inspect order, finalizeDocument reading the HTML the server actually sends, the abort checks after prepare, ready and each inspect, and the bundled imports all check out. The one finding is a target-identity gap for contrast findings that fall back to selector; it is inline.

Scope and verification
  • Read the full non-test diff, plus prepareLiveInspection in inspector/src/live.ts, the fingerprint exclusions in sources.ts, and the runtime target query in core/src/runtime/inspection.ts.
  • Ran a fresh adversarial sub-agent pass.
  • Ran locally on this head:
    • root bun run build: passes;
    • tsc --noEmit on the CLI: passes;
    • bun run lint and oxfmt --check: pass;
    • knip: no new issues compared with the base;
    • the 5 focused test files: 221 passed. The Chromium tests could not run because Chrome will not launch in this sandbox.
  • Bundling: the live-*.js chunk imports only acorn and postcss (both declared CLI dependencies) and sits beside dist/inspection.js. I copied dist into a directory that had only those two packages installed, and prepareLiveInspection loaded and read the runtime.
  • CI: ci.yml only triggers on PRs to main, so Test (cli) has not run on this head. It will run after the PR is retargeted and in the merge queue. Every workflow that did run on this head is green.

Comment thread packages/cli/scripts/build-copy.mjs Resolved
heygengenesis[bot]
heygengenesis Bot previously approved these changes Oct 26, 2026

@heygengenesis heygengenesis Bot 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.

Verdict: APPROVE · 2 resolved, 0 open, 0 new · a003c36 → 6ad5162

Base automatically changed from feat/check-finding-frames-v2 to main October 26, 2026 08:45
@xuanruli
xuanruli dismissed heygengenesis[bot]’s stale review October 26, 2026 08:45

The base branch was changed.

@mintlify

mintlify Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
hyperframes 🟢 Ready View Preview Oct 26, 2026, 8:51 AM

💡 Tip: Enable Automations to automatically generate PRs for you.

@xuanruli

Copy link
Copy Markdown
Contributor Author

Windows render failure (run 40294744314, commit d06422e): this PR caused it, it is not a flake. In the "Build render prerequisites" step, build-copy.mjs failed with ENOENT: no such file or directory, lstat 'D:\a\hyperframes\hyperframes\packages\inspector\dist\inspection.js'. windows-render.yml builds parsers, lint, studio-server, core, player, studio and cli, but not the inspector, and 6ad5162a1 made that copy required. It is the only workflow that builds the CLI without a full bun run build. 0e17b6bdd adds bun run --filter @hyperframes/inspector build before the CLI build. On that commit, Render on windows-latest passed (run 40295855445); only the engine-cli shard was cancelled, by the next push. Against main the diff is only the inspect change plus that one-line workflow fix.

@github-actions

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 149 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

@heygengenesis heygengenesis Bot 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.

Verdict: APPROVE · 0 open, 0 new · 6ad5162 → ba4e334 (now based on main)

Integration check
  • The CLI and docs diff against main is patch-identical to the approved 6ad5162. The only other difference is the package.json version that main bumped.
  • windows-render.yml: inspector now builds before CLI. No other selective CLI build skips it.
  • compositionViewport.ts: calling parseHTML directly matches what the polyfilled DOMParser returned for fragments, full documents and <template>.
  • Locally: tsc passes and 265 focused tests pass. On this head, CI, CodeQL and Windows render all succeeded.

@xuanruli
xuanruli added this pull request to the merge queue Oct 26, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 26, 2026
@xuanruli
xuanruli added this pull request to the merge queue Oct 26, 2026
Merged via the queue into main with commit a39f7b3 Oct 26, 2026
120 checks passed
@xuanruli
xuanruli deleted the feat/check-review-inspect branch October 26, 2026 10:22
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.

1 participant