feat(check): opt-in --layout proseCoverageFloor

[xuanruli]

Summary

• Add --layout "proseCoverageFloor=<0–1>" (semicolon key=value, same grammar as --caption-zone) so callers can lower the prose text_occluded coverage floor without changing OSS defaults.
• Default remains 0.15; omitting --layout is unchanged for everyone else.
• Threads layout.proseCoverageFloor through check parse → browser audit (auditLayout).
• Flag numeric fields (proseCoverageFloor, --frame-check tol, caption fractions) use a shared parseNumberStrict (Number(), not parseFloat) so trailing garbage is rejected.

Zephyr / EF wire contract

Post-release, experiment-framework pins HYPERFRAMES_VERSION and Zephyr CHECK_OPTIONS consume this grammar as a cross-repo contract. The exact argv Zephyr emits today:

--layout proseCoverageFloor=0.05

Built by EF _layout_check_spec as proseCoverageFloor={float:g} from layout_options(prose_coverage_floor=0.05) → CHECK_OPTIONS["layout"]. Future --layout keys should stay semicolon key=value and keep this string parseable (additive fields only; do not rename proseCoverageFloor).

Why

Long titles with padded image overlays can sit at ~0.11 coveredFraction and get discarded under the default floor. Zephyr wants a stricter opt-in (0.05) for scene6-class occlusion without making HyperFrames noisier for other agents.

Test plan

• bun run test src/commands/check.test.ts (strict parse + CLI/driver threading)
• bun run test src/commands/layout-audit.browser.test.ts (~0.07 fixture: default silent, floor 0.05 fires, atomic unchanged)
• Scene 6 dump: default → no text_occluded; --layout proseCoverageFloor=0.05 → text_occluded with coveredFraction: 0.11
• CI green
• After merge: release as next patch (e.g. 0.7.77), then bump EF HYPERFRAMES_VERSION + land Zephyr CHECK_OPTIONS (EF #43493)

[miguel-heygen]

The plumbing is clean and the default-off shape is good: omitting --layout leaves the browser audit on its existing 0.15 default, while the explicit option is threaded through one typed LayoutOptions envelope.

Two changes before I can stamp this:

• [important] packages/cli/src/commands/check.ts:253-257 — reject trailing junk instead of partially parsing it. Number.parseFloat accepts prefixes, so values such as proseCoverageFloor=0.05garbage, 0.1%, and 1e-1oops silently become valid floors. That contradicts the command's advertised fraction grammar and the test name claiming malformed specs are rejected. Parse the whole token (for example with Number(raw) after the existing trim) and pin at least one trailing-junk case.

• [important] packages/cli/src/commands/layout-audit.browser.test.ts:1781-1795 — pin the actual opt-in behavior in the browser audit. The added tests stop at parser → runPipeline options. None executes the changed auditLayout(options) branch, so the core contract in this PR can regress while every new test stays green. Extend the existing ~0.07 prose fixture to prove: default options still produce no text_occluded; proseCoverageFloor: 0.05 produces the finding; and atomic-label behavior remains unchanged. Ideally also retain one driver-level assertion that collectLayout forwards the value into the page evaluation payload.

CI is otherwise green except the still-running Windows test lane at this exact head.

— Magi

Verdict: REQUEST CHANGES
Reasoning: The implementation direction is right, but the new CLI parser accepts malformed values and the browser-side feature itself is not regression-tested.

[xuanruli]

Addressed in 46579aa:

1. Strict parse — parseProseCoverageFloor now uses Number() (same idea as caption fractions), so 0.05garbage / 0.1% / empty reject instead of silently truncating via parseFloat.
2. Browser-audit threshold — pinned the existing ~0.07 (2/27) fixture: default → no prose finding; proseCoverageFloor=0.05 → finding; atomic label still flags either way.
3. Driver forwarding — runScenario(..., { layout: { proseCoverageFloor: 0.05 } }) asserts collectLayout receives the third-arg options.

[james-russo-rames-d-jusso]

Reviewed at 46579aa5fdbd2effec70a4ddbbb531ea4f26fcd1 — Miguel's two [important] asks are addressed in the fix(check): strict proseCoverageFloor parse + pin 0.07 floor tests commit. Layering with the AI-authored / cross-repo lens.

Miguel's asks — closed

• Strict parse (packages/cli/src/commands/check.ts:255-257): Number() instead of Number.parseFloat, plus an empty-string guard. The new test cases at check.test.ts:1159-1161 pin "0.05garbage", "0.1%", and empty string — all reject cleanly. Good.
• Browser-side opt-in behavior test (packages/cli/src/commands/layout-audit.browser.test.ts:1786-1806): two new tests exercise the changed auditLayout(options) branch — prose fires at coveredFraction=0.07 when the floor is lowered to 0.05; atomic-label behavior unchanged. Plus the "ideal also" driver-level assertion Miguel called out lands at check.test.ts:1186-1192 — it("forwards layout options into driver.collectLayout") pins the proseCoverageFloor value all the way to the driver-mock's arg. Cleanly closed.

Fresh concerns

🟡 packages/cli/src/commands/check.ts:216 — sibling parser still has the parseFloat prefix-parse bug. parseFrameCheckField for tol reads const tol = Number.parseFloat(raw), so --frame-check "tol=2px" or tol=4garbage parses to 2 / 4 silently — same class of bug your fix here just corrected for proseCoverageFloor. If the pattern is worth codifying (I'd argue yes), extracting a parseFraction(raw, min, max) / parseNumberStrict(raw) helper and reusing it across parseFrameCheck and parseProseCoverageFloor would carry the fix everywhere in one hop. Not blocking this PR — the immediate goal is scene-6 unblock — but the follow-up is worth naming.

🟡 Cross-repo wire contract. The PR body's "After merge" plan says the CLI will be released as 0.7.77, then EF bumps HYPERFRAMES_VERSION and Zephyr updates CHECK_OPTIONS. That makes the exact --layout grammar a wire contract for at least those two consumers. Two suggestions:

1. Document the canonical string Zephyr will emit in the PR body (e.g. --layout "proseCoverageFloor=0.05" — quoted-single-token, no semicolons yet). Future flag additions (proseCoverageFloor=0.05;anotherField=X) should stay backwards-compatible with the single-field form Zephyr will hardcode.
2. Consider whether EF and Zephyr should pin CLI version in a way that fails loud if the CLI upgrades past the wire contract. Not this PR's concern, but worth naming so the multi-repo coordination doesn't become silent.

Per [[wire-contract-summary-for-coordinated-multi-repo]] — since this is the CLI slice of an FE↔BE↔CLI series, leading with the DTO summary (the exact quoted flag string) in the PR body makes downstream slices' review much easier.

Nits

• packages/cli/src/commands/layout-audit.browser.js:1420-1423 — the browser-side clamp Math.min(1, Math.max(0, options.proseCoverageFloor)) is now redundant with the CLI validation (parseProseCoverageFloor rejects <0 || >1). Not a bug — defensive against non-CLI callers of __hyperframesLayoutAudit — but the comment could mention that the audit is also invoked from browser dev-tools / scratch runs so the clamp isn't dead code.
• Grammar shape: this uses the semicolon-key=value grammar mirroring --caption-zone, but --layout currently has only one field. Fine — future-oriented — but if that stays true for a while, the single-field cost is a minor UX friction (users typing --layout proseCoverageFloor=0.05 vs a bare --prose-coverage-floor=0.05).

What I didn't verify

• Full scene-6 dump on a real project — trusted the PR body's manual verification.
• Zephyr's actual CHECK_OPTIONS shape — I don't have visibility into that repo from here.
• The CLI-side parseCaptionZone sibling — same parseFloat pattern? Skipped for scope; worth a follow-up sweep.

LGTM from my side once Miguel's stamps this — leaving as a comment. Approval's Miguel's.

— Review by Rames D Jusso

[miguel-heygen]

R2 at 46579aa5f closes both prior findings cleanly.

• packages/cli/src/commands/check.ts:253-259 now parses the entire token with Number(), explicitly rejects the empty string, and pins the trailing-junk cases that previously slipped through.
• packages/cli/src/commands/layout-audit.browser.test.ts:1781-1806 now locks the actual browser contract: ~0.07 prose stays quiet at the 0.15 default, becomes text_occluded at a 0.05 floor, and atomic-label behavior remains unchanged.
• packages/cli/src/commands/check.test.ts:1184-1192 additionally pins the pipeline → driver forwarding seam.

Local verification at this exact head: full workspace build green; targeted browser-audit suite 78/78; targeted check suite 44/44. The rewritten-head CI matrix is still running, with no failures at review time.

— Magi

Verdict: APPROVE
Reasoning: Both requested correctness/test gaps are fixed and pinned at the exact head; no remaining code blocker found.

[miguel-heygen]

R3 at a0d068ab3 closes the follow-up parser nit cleanly.

• packages/cli/src/commands/check.ts:216-218 now routes --frame-check tol through the same strict whole-token parser as proseCoverageFloor, so tol=4px and tol=2garbage no longer prefix-parse.
• packages/cli/src/commands/check.ts:242-247 centralizes the invariant in parseNumberStrict; caption fractions reuse it without changing their already-strict behavior.
• packages/cli/src/commands/check.test.ts:1150-1151 positively pins both malformed tolerance cases.

Local exact-head verification: check.test.ts 45/45; formatting and lint clean. CI has no failures; the two Windows jobs and CLI smoke are still running at review time.

— Magi

Verdict: APPROVE
Reasoning: The requested sibling-parser cleanup is minimal, shared, and regression-pinned; no new blocker found.