Skip to content

fix(parse): accept a minimal review object (no summary) as a clean review - #63

Merged
atbe merged 1 commit into
mainfrom
fix/parse-minimal-review
Oct 4, 2026
Merged

atbe merged 1 commit into
mainfrom
fix/parse-minimal-review

Conversation

@atbe

@atbe atbe commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Why

A reviewer on monorepo #7104 failed with ⚠️ ... Harness output is not a review. The harness (glm-5.3), having nothing to report, emitted:

{"highlights":[],"diagram":""}

a review-shaped object with no summary. The parser hard-required a string summary and rejected the whole thing, failing the reviewer instead of treating it as an empty clean review.

Fix

"Recognizable review" now means an object carrying at least one review key (summary/findings/concerns/highlights/diagram). A missing summary defaults to "" (the schema already defaults the arrays), so a minimal object = empty clean review. Guardrails kept: unrelated JSON like {"status":"done"} (no review key) still fails, and a non-string summary still fails.

task check green (494 tests, +5 parse cases). Consumers get it via @main.

🤖 Generated with Claude Code

…view

A reviewer on monorepo #7104 failed with "Harness output is not a review"
because glm-5.3, having nothing to report, emitted `{"highlights":[],
"diagram":""}` — a review-shaped object with no `summary`. The parser
hard-required a string `summary` and rejected the whole review, failing
the reviewer.

Loosen "recognizable review" to: an object carrying at least one review
key (summary/findings/concerns/highlights/diagram). A missing summary
defaults to "" (schema already defaults the arrays), so a minimal object
is an empty clean review. Unrelated JSON like {"status":"done"} carries
no review key and still fails; a non-string summary still fails.

task check green (494 tests, +5 parse cases).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown

🔍 Loupe review


code

✅ no issues · 1 file

Relaxes parseReviewOutput's shape gate from 'string summary required' to 'at least one review key present', defaulting a missing summary to ""
via the existing schema. The minimal {"highlights":[],"diagram":""} output now parses as an empty clean review while guardrails (no review key → throw; non-string summary / non-array findings → throw) are preserved. Checked the schema and all parse callers (produceReview's .then(parseReviewOutput), summary rendering in github.ts) — safe.

Run details
  • review: agentic
  • verification: skipped
  • scope: full
  • dropped: 0 malformed finding(s), 0 malformed concern(s), 0 out of scope, 0 below profile, 0 rejected by verification, 0 demoted by the comment cap, 0 duplicate of another reviewer
  • off-diff notes published: 0

Last reviewed commit: 7694a86


docs

✅ no issues · 1 file · ⚠️ degraded run

This PR relaxes parseReviewOutput in packages/core/src/parse.ts so a harness object carrying any review key (summary/findings/concerns/highlights/diagram) counts as a recognizable review, with summary now optional and defaulting to "" — fixing spurious "Harness output is not a review" failures for empty clean reviews like {"highlights":[],"diagram":""}. Verdict: no docs impact. This touches only the internal acceptance check for harness output — no Action input, .loupe.json field, CLI flag, built-in default, posted GitHub object, prompt contract, or chat command changes. The review JSON key set is unchanged (highlights and diagram were already part of the parsed/validated shape — the destructuring of them at the bottom of the hunk shows they already existed downstream); the PR only loosens what the parser accepts, and schema validation of the actual field types still applies after parsing. Error-message text is consumer-visible only in a failure path, not a documented contract. Note: I ran headless with no repository access, so I could not grep docs/** for a page that documents the harness-output parse contract; nothing in the diff itself contradicts any shipped documentation, so I am not raising a finding I cannot anchor.

Run details
  • review: headless fallback (agentic run failed)
  • verification: skipped
  • scope: full
  • dropped: 0 malformed finding(s), 0 malformed concern(s), 0 out of scope, 0 below profile, 0 rejected by verification, 0 demoted by the comment cap, 0 duplicate of another reviewer
  • off-diff notes published: 0

Last reviewed commit: 7694a86


Use @loupe fix to address all open findings.

@atbe
atbe merged commit 258e808 into main Oct 4, 2026
2 checks passed
@atbe
atbe deleted the fix/parse-minimal-review branch October 4, 2026 00:04
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