-
Notifications
You must be signed in to change notification settings - Fork 2
Report per-skill turns and cost in PR body + dispatch summary #781
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -641,6 +641,35 @@ jobs: | |||||
| NO_CHANGES.md at repo root with a one-line explanation. | ||||||
| Still do not hand-edit any file. | ||||||
|
|
||||||
| # Capture skill_gen's execution stats BEFORE skill_review runs | ||||||
| # and overwrites the shared execution-output JSON at the | ||||||
| # canonical claude-code-action location. Lets us report | ||||||
| # per-invocation turns/cost in the PR body and the workflow_ | ||||||
| # dispatch summary comment. Missing-file defaults to 0 so a | ||||||
| # failed run still emits plausible outputs. | ||||||
| - name: Capture skill_gen stats | ||||||
| id: skill_gen_stats | ||||||
| if: always() && steps.skill_gen.conclusion == 'success' | ||||||
|
||||||
| if: always() && steps.skill_gen.conclusion == 'success' | |
| if: always() && steps.skill_gen.conclusion != 'skipped' |
Copilot
AI
Apr 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue as skill_gen_stats: this capture step only runs when steps.skill_review.conclusion == 'success', so it won’t emit outputs for failed/cancelled runs even though the script has a missing-file fallback to 0. If the goal is to report stats best-effort regardless of success, consider allowing this step to run for any non-skipped conclusion (or adjust the comments/expectations accordingly).
| if: always() && steps.skill_review.conclusion == 'success' | |
| if: always() && steps.skill_review.conclusion != 'skipped' |
Copilot
AI
Apr 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the workflow_dispatch summary table, the Cost cells use ${GEN_COST:+...} / ${REVIEW_COST:+...} / ${TOTAL_COST:+...}. When the variable is unset/empty this expands to an empty string (not the – placeholder), so the markdown table will have blank cells and won’t match the stated “placeholders so all rows have 4 cells” behavior. Consider defaulting to an explicit placeholder (and only adding the $ prefix when a value is present).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in the comment:
workflow_reads like an accidental truncation and is confusing in context. Consider changing it toworkflow_dispatchto match the actual event name referenced elsewhere in this workflow.