[codex] fix resilient coordinator triage output#458
Conversation
e75c50d to
2df343a
Compare
quangdang46
left a comment
There was a problem hiding this comment.
π€ Hermes QA Report | PR #458 β [codex] fix resilient coordinator triage output
Verdict: APPROVED β β Code is well-structured, properly tested, and addresses a concrete silent-noop bug.
Quality Gates
| Check | Result |
|---|---|
go vet ./... |
β PASS |
go build ./... |
β PASS |
go test ./internal/coordinator/triage/... |
β PASS (15/15) |
Code Review Summary
What this PR does (3 changes in 2 files, +197/β20):
-
Resilient JSON parsing β Adds
extractJSON()that strips markdown fences (```json) and prose wrapping beforejson.Unmarshal, so the coordinator no longer silently no-ops when LLMs return fenced or prose-wrapped JSON output. -
Graceful defaulting β Replaces strict
requireExactlyOne()withrequireAllowedOrDefault()that tolerates missing label fields by falling back to sensible defaults (kind β kind/feature, area β area/runtime for bugs, dispatch β dispatch/plan, complexity β complexity/m). Labels from out-of-scope/unclear dispositions are now ignored instead of rejected. -
Warmer comment style β Added maintainer-style comment guidance to the prompt with reporter @-mention, concrete issue reference, clear next steps. Added
area/sweeperto allowed areas.
Positives:
- Properly tested: 133 new lines of tests covering fenced JSON, text-wrapped JSON, missing labels, extra labels, out-of-scope ignore, first-allowed selection, and warm-style prompt guidance. All existing tests preserved.
- Clean design: The
extractJSONβrequireAllowedOrDefaultchain is simple, composable, and easy to verify. No new abstractions introduced. - Well-scoped: 197 additions focusing only on the triage output parsing layer. No scope creep.
- PR body is substantive: Root cause analysis, validation with live testing on 5 repos, and concrete examples of what broke.
Concerns:
- None. Code is clean, tests are thorough, changes are minimal and well-justified.
Recommendation
Ready to merge. The upstream origin/main has drifted significantly (137 file diff from branch base), so the PR will need a rebase/merge before landing, but the code itself is clean.
Reviewed at 2026-06-21 from Hermes QA cron.
Fixes #456
Summary
Root cause
The coordinator treated the LLM response as exact raw JSON and rejected otherwise usable decisions when models returned fenced JSON, prose-wrapped JSON, missing label groups, extra labels on non-valid dispositions, or more than one dispatch label. That made triage silently no-op in common agent-output cases.
Validation
Notes
I sampled recent lefarcen comments on nexu-io/open-design issues nexu-io/open-design#2815, nexu-io/open-design#2801, nexu-io/open-design#2794, and nexu-io/open-design#2785. The common pattern was warm acknowledgement, concrete issue recap, explicit routing/status, and a clear next step; this PR adds that guidance to the coordinator prompt without changing the comment stamping/disclosure path.