[jongio]

Projects per-eval and per-case quality scores, thresholds, trial durations, and token usage (input/output/total) from Vally JSONL into the structured report, GitHub Actions Job Summary, and sticky PR comment for marketplace PR review.

Validation

• pnpm check (typecheck, lint, test:coverage 168/168, check:dist, test:packaged): Passed

[Copilot]

🟡 Changes recommended

Raw stimulus names are published to artifacts, job summaries, and PR comments without explicit opt-in, risking disclosure of private case descriptions, prompts, or source paths.

5 open issues

• Hide raw stimulus names unless explicitly opted in · New
• Truncate report rows to fit the comment byte limit · New
• Budget rendered summaries against the 1 MiB limit · New
• Keep report artifact size within the reader limit · New
• Reject more cases than executed stimuli · New

What changed in this PR

Extends the Vally JSONL reporting adapter to surface quality scores, duration, token usage, and case outcomes in structured artifacts, Job Summaries, and sticky PR comments.

Changes:

• Projects optional metrics and bounded case summaries from native evidence.
• Adds structured-report validation and quality tables to both GitHub presentation surfaces.
• Adds projection and rendering tests, and clears aggregate metrics before execution.

File	Description
tests/​reporting.test.ts	Tests metric round-tripping, validation, and summary rendering.
tests/​reporter.test.ts	Tests quality tables in sticky PR comments.
tests/​native.test.ts	Tests native metric and case projection.
src/​reporting.ts	Adds metric types, serialization, validation, and summary tables.
src/​reporter.ts	Adds quality and token tables to PR comments.
src/​native.ts	Collects scores, durations, tokens, and case outcomes.
src/​action.ts	Clears duration and token totals before execution.

_{Review effort: Balanced (auto)}

[wbreza]

Requesting changes on #24 at 0d1ba8d.

• High: Keep private case names out of default reports unless explicitly opted in. Existing disclosure thread.
• Medium: Preserve Vally's threshold-based case outcomes instead of showing a passing trial as failed. New inline finding.
• Medium: Bound the added details so large suites do not lose their PR comment, job summary, or readable report artifact.
• Medium: Reject case results when no corresponding stimuli ran, rather than displaying a passing case in a not-run report. Existing validation thread.

Read-only review: no PR scripts or tests were run, and the bundled build was not independently rebuilt. The PR also has merge conflicts. The failed security-review check exceeded its input-size limit; that is an incomplete check, not evidence of another code defect, so there is no code finding to anchor inline.

Generated by 🌳Breza Bot

[github-actions]

Evals: passed

Suite: copilot-smoke

Lint: validated (no model execution) - Evidence

Run: passed - 1 trial - Evidence

Eval	Trials	Result
copilot-skill-smoke	1	passed

Quality & token summary

Eval	Verdict	Score	Duration	Tokens
copilot-skill-smoke	passed	1.00 (threshold 1.00)	2080ms	33027 tokens (32954 in / 73 out)

Exact revision | Workflow run

[wbreza]

Requesting changes on #24 at dca4550. Four earlier findings are addressed in code: case-name disclosure, case verdicts, impossible case counts, and oversized job summaries. The merge conflict is also gone.

Two medium findings remain:

• Complete-comment size: the new section fits its own budget but can still push the full comment over 60,000 bytes, failing reporting.
• Artifact size: the writer can still produce a valid large report that the reader rejects. Budget the optional details or align the limits.

Current-head lint, unit tests, type checking, bundle verification, and smoke-test checks passed on GitHub. I did not run PR scripts or tests locally; the boundary evidence comes from standalone size calculations. The security-review check still failed because its input exceeded its size limit, leaving that check incomplete rather than proving another code defect. No new code thread is appropriate for that check failure.

Generated by 🌳Breza Bot

[wbreza]

Requesting changes on #24 at f4f3166. The two remaining size regressions are addressed, so all six earlier findings are now addressed in code.

One medium regression remains: partial metrics are rejected. When only some eval groups report duration or tokens, the writer correctly leaves the overall total unknown, but the new validation rejects that report. Keep known group metrics without requiring an overall total, and cover that valid round trip.

Current-head lint, unit tests, type checking, bundle verification, and smoke-test checks passed on GitHub. I did not run PR scripts or tests locally. The security-review check still exceeded its input-size limit; that leaves the check incomplete, not evidence of another code defect or a new inline finding.

Generated by 🌳Breza Bot

[Copilot]

🟡 Changes recommended

The validation guard rejects valid artifacts with partial duration or token coverage and omitted run totals, preventing the report command from publishing its PR comment.

1 open issue

• Accept matching undefined totals for incomplete metric coverage · New

5 resolved since last review

• Hide raw stimulus names unless explicitly opted in
• Reject more cases than executed stimuli
• Keep report artifact size within the reader limit
• Budget rendered summaries against the 1 MiB limit
• Truncate report rows to fit the comment byte limit

_{Review effort: Balanced (auto)}

[wbreza]

Approved #24 at cfd59d4. All seven tracked code findings are addressed. The final fix accepts unknown overall totals when only some groups have metrics, while still rejecting incorrect totals; the partial-metrics finding now has a round-trip regression test.

Current-head GitHub checks passed for all 180 unit tests, lint, type checking, bundle verification, smoke tests, and CodeQL. I did not run PR scripts or tests locally.

The separate security-review check still failed because its input exceeded its size limit. That check remains incomplete; this code-review approval does not mean every CI check passed. No additional code defect or inline finding is established by that tool failure.

Generated by 🌳Breza Bot

[Copilot]

🟡 Changes recommended

The metrics-free fallback can exceed the parser’s 1,000,000-byte limit with valid group names, causing the separate report job to reject the artifact and preventing publication.

1 open issue

• Metrics-free fallback can exceed the parser byte limit and block publishing · New

1 resolved since last review

• Accept matching undefined totals for incomplete metric coverage

_{Review effort: Balanced (auto)}

[wbreza]

Changing my earlier approval to request changes on #24 at cfd59d4. I missed a remaining artifact-size case that the new review identified.

Medium: The metrics-free fallback can still exceed the reader's limit. I independently calculated a valid 1,000-group report with JSON-escaped control characters in its names: 1,032,378 bytes after removing group metrics. That fits the previous 1 MiB file limit but exceeds the new 1,000,000-byte parser limit, so the report job rejects it. Check and bound the final fallback too, preserving native outcomes and counts.

The other six tracked findings remain addressed. This verification used source inspection and a standalone size calculation, not execution of PR scripts or tests. The previously noted security-review check remains a separate input-size limitation, not the basis for this code finding.

Generated by 🌳Breza Bot

[wbreza]

Approved #24 at 6c6c97e. The remaining artifact fallback issue is addressed: the writer checks the metrics-free report and omits optional names if it is still too large, preserving group identities, outcomes, and counts. The escaped-name boundary case now has a round-trip test. All seven tracked findings are addressed in code.

Current-head GitHub checks passed for all 180 unit tests, lint, type checking, bundle verification, smoke tests, and CodeQL. I did not run PR scripts or tests locally.

The separate security-review check still exceeded its input-size limit, and another automated review was queued when checked. This approval does not mean every automated check has completed successfully.

Generated by 🌳Breza Bot

[Copilot]

🟡 Changes recommended

The new metrics lack real Vally/mock-executor coverage and structured-report round-trip assertions, so native-output mismatches could silently omit metrics without failing tests.

1 open issue

• Add native metrics assertions to the model-free CLI test · New

1 resolved since last review

• Metrics-free fallback can exceed the parser byte limit and block publishing

_{Review effort: Balanced (auto)}

[wbreza]

Changing my earlier approval to request changes on #24 at 6c6c97e for one verified acceptance-coverage gap. The seven previous code findings remain addressed.

Medium: Add model-free native metrics coverage. The new metrics assertions use hand-written JSONL; the real Vally/mock-executor tests assert outcomes, counts, and names, but not the new metrics or their structured-report round trip. This misses the repository's requirement to qualify native behavior with the built-in mock executor. Extend that existing test to check score/threshold, duration, and native token-usage availability, preserving unknown usage when absent.

This is a demonstrated test-coverage gap, not a claim that the current native metrics are wrong. The passing unit tests and model-backed smoke run do not fill that model-free acceptance gap. I inspected the tests and packaged checks but ran no PR scripts or tests locally. The separate security-review tool's input-size failure is unchanged and is not the basis for this request.

Generated by 🌳Breza Bot

[wbreza]

Approved #24 at 2317895. All eight tracked findings are addressed. The model-free coverage gap is closed by assertions against the real Vally/mock-executor output and a structured-report round trip. The test verifies scores, thresholds, duration, and the pinned mock executor's zero token usage without running a model.

Current-head GitHub CI passed all 26 native tests and all 180 tests overall, plus lint, type checking, bundle verification, smoke tests, and CodeQL. I inspected the changes and CI logs; I did not run PR scripts or tests locally.

The separate security-review check still exceeded its input-size limit, and another automated review was in progress when checked. This code-review approval does not mean every automated check has completed successfully.

Generated by 🌳Breza Bot

[Copilot]

🟢 Approval recommended

No active review issues remain for the native reporting metrics and bounded-output changes.

No open issues 🎉

1 resolved since last review

• Add native metrics assertions to the model-free CLI test

_{Review effort: Balanced (auto)}