Skip to content

Fix stats parse + add Summary of changes section#785

Merged
rdimitrov merged 2 commits intomainfrom
fix-execution-output-jq-parse
Apr 22, 2026
Merged

Fix stats parse + add Summary of changes section#785
rdimitrov merged 2 commits intomainfrom
fix-execution-output-jq-parse

Conversation

@rdimitrov
Copy link
Copy Markdown
Member

@rdimitrov rdimitrov commented Apr 22, 2026

Two changes driven by findings from the v4 e2e test on PR #784.

1. Fix jq parse of claude-execution-output.json

The execution log is a top-level JSON array of messages (init, assistant turns, tool calls, result), not a single summary object. My previous query `.num_turns // 0` fails with `Cannot index array with string` on that format, causing `skill_gen_stats` and `skill_review_stats` steps to error, which cascaded to:

  • Workflow conclusion `failure` despite skill, autofix, and CI all succeeding
  • Summary comment missing cost/turns prefix (rendered as just "Done · [run]")
  • PR body missing the Run cost section

Fix: recursive descent + last-match, handling both array and object shapes. `|| echo 0` guards against future format breaks.

```bash
TURNS=$(jq -r '[.. | .num_turns? // empty] | last // 0' "$LOG" 2>/dev/null || echo 0)
```

Dry-run validated against both shapes (old-object and new-array).

2. Add Summary of changes section to PR body

V4's PR #784 rendered the At-a-glance table with `1 commit(s)` but no indication of WHAT that commit contained. Reviewers had to click through to the Commits tab or open the diff. New section fixes that with two sources:

Skill-written SUMMARY.md (preferred)

New prompt instruction at the end of Phase 6 tells `skill_gen` to write `SUMMARY.md` at repo root with a 3-8 bullet description of changes. Format guides the skill toward reviewer-useful summaries:

  • Added `v1alpha1 → v1beta1` migration guide at `docs/toolhive/guides-k8s/migrate-to-v1beta1.mdx`
  • Updated `auth-k8s.mdx` with the default `redirectUri` behaviour new in v0.23.1
  • Swept v1alpha1 apiVersion references across 37 files in `guides-k8s/`, `guides-vmcp/`, `tutorials/`
  • Added enterprise admonition to Cedar policy reference for upgrade warnings

`skill_review` is told not to touch `SUMMARY.md`.

Commit-title fallback (defensive)

If `SUMMARY.md` is missing, the signals step extracts skill-commit titles via `git log --reverse $BASELINE..HEAD` and renders as a bullet list. Prefixed with `Auto-generated from commit titles - skill did not write SUMMARY.md.` so reviewers know the origin.

Placement

Rendered as `### Summary of changes` in the PR body, directly after the At-a-glance table (above Gaps / contributors / cost / details). Skipped entirely on silent runs and NO_CHANGES.md cases — no content, no summary.

Why now

From the v4 audit:

are you sure this PR is all the fixes we need to make it work as test runs are expensive (each is $20), I noticed we don't have the cost summary or any other meaningful summary

#1 fixes the cost summary. #2 adds the "other meaningful summary" — the actual content description, not just a commit count.

Expected behaviour after merge

Next `workflow_dispatch` on real content should produce a PR body with:

```

Docs update for toolhive v0.23.1

At a glance

[table with real numbers]

Summary of changes

  • Added v1beta1 migration guide at
  • Updated auth-k8s with default redirectUri section
  • ...

Run cost

[table: turns + cost per session + total]

Details...
\`\`\`

Plus a summary comment: `Done · 152 turns · $10.76 · run · see PR body for details`

claude-code-action writes a top-level JSON array of messages
(init, assistant turns, tool calls, result). The final entry
is a "result" object with the session summary. My previous
jq query `.num_turns // 0` assumed a top-level object and
failed with "Cannot index array with string" on v4 test run
24775890520, cascading both `skill_gen_stats` and
`skill_review_stats` steps to failure and leaving the
workflow's overall conclusion as "failure" despite every
other step (including the skill itself, autofix, and CI
lint) succeeding cleanly.

Fix: use recursive descent + `last` to pick up num_turns /
total_cost_usd / permission_denials wherever they appear, so
the query survives both the current array format and any
future structure shift. `|| echo 0` guards against future
format breaks.

Dry-run verified against both array and object shapes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 22, 2026 11:59
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 22, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs-website Ready Ready Preview, Comment Apr 22, 2026 0:15am

Request Review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the GitHub Actions workflow’s parsing of claude-execution-output.json by handling the current upstream format where the log is a top-level JSON array (rather than a single object), ensuring per-skill turns/cost/denials stats are still captured correctly.

Changes:

  • Update jq extraction for num_turns, total_cost_usd, and permission_denials_count to work with both array- and object-shaped JSON.
  • Add inline documentation explaining the upstream log shape and the error-guarding approach.
  • Apply the same fix to both the generation and review stats capture steps.

Two new sources of content-change summary in the PR body, so
reviewers can skim what shipped without clicking into the diff:

1. **Skill-written SUMMARY.md** (Option B): new prompt
   instruction at the end of Phase 6 tells skill_gen to write
   SUMMARY.md at repo root with a 3-8 bullet description of
   changes. Format guides the skill toward reviewer-useful
   summaries ("Added X at <path>", "Swept v1alpha1->v1beta1
   across N files"), not enumeration. skill_review is told not
   to touch SUMMARY.md.

2. **Commit-title fallback** (Option A): if SUMMARY.md is
   missing, the signals step extracts skill commit titles via
   `git log --reverse $BASELINE..HEAD` and formats as bullets.
   Prefixed with a "_Auto-generated from commit titles_" note
   so reviewers know it wasn't hand-curated.

Rendered as a new "### Summary of changes" section in the PR
body, placed directly after the At-a-glance table (above Gaps /
contributors / cost / details).

Skipped entirely on silent runs and NO_CHANGES.md runs -- no
content, no summary.

Validation: on a real content run like v4 would have been, the
PR body will now show:

  ### Summary of changes
  - Added v1alpha1 -> v1beta1 migration guide at <path>
  - Updated auth-k8s with default redirectUri section
  - Swept apiVersion references across N files in guides-k8s/

...or the commit-title fallback if SUMMARY.md wasn't produced.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@rdimitrov rdimitrov changed the title Fix jq parse of claude-execution-output.json (array format) Fix stats parse + add Summary of changes section Apr 22, 2026
@rdimitrov rdimitrov enabled auto-merge (squash) April 22, 2026 12:21
@rdimitrov rdimitrov merged commit 8474e6d into main Apr 22, 2026
3 checks passed
@rdimitrov rdimitrov deleted the fix-execution-output-jq-parse branch April 22, 2026 12:22
rdimitrov added a commit that referenced this pull request Apr 22, 2026
PR #788's skill run committed SUMMARY.md to the branch alongside
its content commit (42267da) because /SUMMARY.md isn't in
.gitignore -- `git add -A` during claude-code-action's auto-commit
swept it up. The workflow's signals step reads and removes
SUMMARY.md, but only from the working tree; the commit that added
it sticks around.

/GAPS.md and /NO_CHANGES.md are already gitignored for exactly
this reason; SUMMARY.md was added in #785 but I missed adding the
matching gitignore entry.

With this in place, claude-code-action's `git add -A` will skip
SUMMARY.md, the skill still writes it to disk, the signals step
still reads + deletes it -- no commit involvement at all, which
is how GAPS.md and NO_CHANGES.md already work.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

3 participants