Skip to content

[rig-sampler] docs(rig): fix misleading analyzeResponse header comments and add JSDoc#62

Merged
pelikhan merged 1 commit into
mainfrom
rig-sampler/53-ralf-loop-ad3f240e1ea4511b
Jul 23, 2026
Merged

[rig-sampler] docs(rig): fix misleading analyzeResponse header comments and add JSDoc#62
pelikhan merged 1 commit into
mainfrom
rig-sampler/53-ralf-loop-ad3f240e1ea4511b

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Summary

Fixes two incorrect documentation comments in skills/rig/rig.ts about how analyzeResponse extracts JSON from model responses, and adds an accurate JSDoc block on the exported function.

Samples run

Sample Result Notes
53-ralf-loop.ts ✅ pass (3 tests) Sub-agent RALF loop with diagnose+fix agents. Stub ran 2 rig.agent.ask events, 1 turn each, no repairs needed. Clean schema fit.
54-large-scale-summarization-rigs.ts ✅ pass Fan-out shard-summary pattern with planDeterministicSearch/summarizeShard/reduceScenario sub-agents. 6 rig.agent.ask events emitted; stub correctly synthesized array outputs from declared schemas.
55-file-change-lint-middleware.ts ⚠️ stub passes, markdown typecheck fails Sample imports zx (not a rig dep). Stub run completed fine, but the skills/rig/samples typecheck suite catches it. The run highlighted how AgentAddon typing allows zx-style async middleware cleanly.
56-single-agent-sonnet.ts ✅ pass Minimal single agent with refactorOne sub-agent. One rig.agent.ask event, stub output matched {file:string, change:string} schema.
57-complex-integration-sonnet.ts ✅ pass Issue-review-port with {summary, questions} output. One turn, no repairs. Schema felt well-matched to the task.

What the run revealed

While examining analyzeResponse to understand why stub outputs were being accepted without any <output> XML parsing, I found two comments in the file-header block that were actively misleading:

* F:analyzeResponse(resp,schema,name,turn) ResponseAnalysisResult parse+validate from <output> XML tag
* INV:output-tag model response parsed from <output>...</output> XML tag in assistant message

The actual implementation (analyzeResponseparseJson) never looks for an <output> tag. It uses three strategies in order:

  1. Parse the entire response as JSON.
  2. Extract and parse the first ```json ... ``` fenced block.
  3. Extract and parse the first balanced {...} or [...] value.

This comment misleads contributors into thinking they need to wrap responses in <output> tags (or that the prompt somehow asks for them), which is not the case.

Change

  • Corrected the two header comment lines to accurately describe the three-strategy JSON extraction.
  • Added a JSDoc block directly on analyzeResponse documenting the strategy order and return shape, consistent with other documented exports like configureAgent and agent.

No logic changes; all 156 tests pass.

Generated by Daily Rig Sampler · sonnet46 87.2 AIC · ⌖ 8.14 AIC · ⊞ 5.4K ·

The file-header invariant comment claimed the model response was parsed
from an '<output>...</output>' XML tag, but the actual implementation
in analyzeResponse/parseJson never extracts such a tag — it parses
JSON directly from the raw response text with three fallback strategies
(direct parse → fenced ```json block → balanced-brace extraction).

Fix the two incorrect header lines and add a JSDoc comment on
analyzeResponse that accurately documents the three-strategy extraction
pipeline so callers understand how repair prompts are triggered.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@pelikhan
pelikhan marked this pull request as ready for review July 23, 2026 16:54
@pelikhan
pelikhan merged commit ea38d7c into main Jul 23, 2026
1 check passed
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skills-Based Review

Applied /grill-with-docs — the corrections are accurate and the JSDoc is well-formed.

📋 Highlights

Positive Highlights

  • ✅ Both header comment lines now accurately describe the three-strategy JSON extraction (direct parse → fenced block → balanced-brace)
  • ✅ JSDoc on analyzeResponse is clear, structured, and consistent with other documented exports
  • ✅ The misleading <output> XML tag references are cleanly removed
  • ✅ No logic changes; all tests pass

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 17.3 AIC · ⌖ 4.04 AIC · ⊞ 6.3K
Comment /matt to run again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant