fix: improve weekly metrics workflow with 14-day window and debug output#1226
fix: improve weekly metrics workflow with 14-day window and debug output#1226Crunchyman-ralph merged 19 commits intonextfrom
Conversation
- Extend search window from 7 to 14 days for more reliable metrics - Add debug output to troubleshoot empty metrics - Add fallback values when no metrics files found - Show date range in Discord message for clarity
|
| Name | Type |
|---|---|
| @tm/cli | Patch |
| extension | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
|
Warning Rate limit exceeded@Crunchyman-ralph has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 14 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughReplaces inline shell parsing in the weekly GitHub Actions workflow with a Node script that reads Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Node as parse-metrics.mjs
participant FS as File system
participant DC as Discord webhook
GH->>GH: compute first_day/last_day → date_range ("Past 14 days ...")
GH->>Node: run node .github/scripts/parse-metrics.mjs (env: date_range)
Node->>FS: read `issue_metrics.md` and `pr_metrics.md` (if present)
alt files present
Node->>Node: parse counts and avg metrics (issues/pr)
else missing
Node->>Node: log missing and set safe defaults (0 / N/A)
end
Node-->>GH: emit key=value lines and append to GITHUB_OUTPUT (if set)
GH->>DC: POST Discord message (includes date_range + updated metrics fields + visual analytics link)
DC-->>GH: HTTP response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
- Use 'Total number of items created' instead of 'issues/pull requests created' - Use 'Number of items closed' instead of 'issues/pull requests closed' - Patterns now match the actual output from github/issue-metrics@v3
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/weekly-metrics-discord.yml(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (3)
.github/workflows/weekly-metrics-discord.yml (3)
22-24: 14‑day window change looks good; verify “Week of” label consistency and TZ.
- Window extension is correct and
date_rangeaddition is clear. However,week_ofstill derives from “7 days ago” (Line 28, unchanged). Consider aligning it tofirst_dayor drop it from the message to avoid conflicting period labels. Also confirm intended timezone (runners are UTC); set TZ if you need a specific zone.Also applies to: 29-29
54-67: Good fallbacks when issue metrics are missing.Defensive defaults and debug output are appropriate and will prevent step failures when files are absent.
91-96: Helpful final debug summary.The explicit counters will simplify troubleshooting when parsing changes upstream.
- Back to 7-day window now that parsing is fixed - Remove debug output for cleaner workflow logs - Workflow should now show real metrics reliably
- Use awk -F'|' to parse markdown table columns properly - Extract actual numbers instead of table headers - Should now show '7' instead of '| Total number of items created | 7 |'
- Use direct grep for 'Time to first response' and 'Time to close' table rows - Parse table columns with awk -F'|' to get actual time values - Should now show response times instead of N/A
- Create .github/scripts/parse-metrics.mjs for robust markdown parsing - Replace complex bash/awk parsing with proper JavaScript logic - Better error handling and debug output - Should correctly parse time values and handle edge cases
- Add actions/setup-node@v4 with Node 20 - Required to run .github/scripts/parse-metrics.mjs - Following pattern from other workflows that use .mjs scripts
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/weekly-metrics-discord.yml (2)
23-25: Window still 7 days; PR objective says 14.Update the range and label to 14 days.
- # Last 7 days - first_day=$(date -d "7 days ago" +%Y-%m-%d) + # Last 14 days + first_day=$(date -d "14 days ago" +%Y-%m-%d) last_day=$(date +%Y-%m-%d) @@ - echo "date_range=Past 7 days ($first_day to $last_day)" >> $GITHUB_ENV + echo "date_range=Past 14 days ($first_day to $last_day)" >> $GITHUB_ENVAlso applies to: 29-29
39-49: Split created vs merged PR metrics; update parserAdd a second metrics step that queries merged PRs by merged date (SEARCH_QUERY: repo:${{ github.repository }} is:pr is:merged merged:${{ env.first_day }}..${{ env.last_day }}) and outputs pr_merged_metrics.md; update .github/scripts/parse-metrics.mjs to read pr_merged_metrics.md for prs_merged/pr_avg_merge_time (keep existing created-PR step and parser fallback). Files: .github/workflows/weekly-metrics-discord.yml (≈lines 39–49) and .github/scripts/parse-metrics.mjs.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/parse-metrics.mjs(1 hunks).github/workflows/weekly-metrics-discord.yml(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
📚 Learning: 2025-09-09T19:14:21.367Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1194
File: .github/workflows/ci.yml:21-21
Timestamp: 2025-09-09T19:14:21.367Z
Learning: User Crunchyman-ralph considers suggestions about adding safer default shell options (like bash -euo pipefail) to GitHub Actions workflows as too nitpicky and prefers not to implement them.
Applied to files:
.github/workflows/weekly-metrics-discord.yml
🪛 GitHub Actions: Weekly Metrics to Discord
.github/scripts/parse-metrics.mjs
[error] 1-1: Cannot find module '/home/runner/work/claude-task-master/claude-task-master/.github/scripts/parse-metrics.mjs'. Process completed with exit code 1. Ensure the script exists or correct the path.
🔇 Additional comments (2)
.github/workflows/weekly-metrics-discord.yml (2)
31-39: Pin the issues output filename to match the parser.The script reads issue_metrics.md; make the generator explicit to avoid default drift.
- name: Generate issue metrics uses: github/issue-metrics@v3 env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} SEARCH_QUERY: "repo:${{ github.repository }} is:issue created:${{ env.first_day }}..${{ env.last_day }}" HIDE_TIME_TO_ANSWER: true HIDE_LABEL_METRICS: false + OUTPUT_FILE: issue_metrics.md
59-60: Nice touch adding the date range line.Consistent with the PR objective; once the window is 14 days, this will read correctly.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/scripts/parse-metrics.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
🪛 GitHub Actions: CI
.github/scripts/parse-metrics.mjs
[error] 1-1: Formatter would have reformatted the file. Please run 'biome format .' to fix code style issues.
🔇 Additional comments (4)
.github/scripts/parse-metrics.mjs (4)
3-4: Consolidate fs imports.Minor cleanup; fewer import statements.
6-20: Harden table parsing against column drift and label variants.Relying on "index 2" is brittle and "includes()" can misfire. Match the row by name and capture the first value cell.
22-27: Make numeric extraction robust (commas/units).parseInt on raw cells breaks on "1,234" or "123 items". Strip non‑digits first.
62-62: Fix "merged vs closed" mismatch for PR stats.You label outputs as "merged" but parse "closed" and "time to close". This skews the Discord report.
- Increase window from 7 to 14 days for better sample sizes - Remove debug output now that we understand the metrics - Should give more representative averages for response/merge times
- Include Repobeats chart URL in the weekly metrics message - Provides visual complement to the text-based metrics - Shows broader repository health and trends
- Consolidate fs imports into single import statement - Harden table parsing with better column detection and filtering - Improve numeric extraction to handle commas and edge cases - Add comprehensive error handling for file writes - Add stdout logging for better debugging and transparency - Add file existence logging with emojis for clarity
- Add 'set -Eeuo pipefail' for strict bash error handling - Make issue metrics output file explicit to avoid coupling to action defaults - Prevents future breakage if github/issue-metrics changes default filenames - Follows defensive programming best practices
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/weekly-metrics-discord.yml (2)
9-13: Tighten permissions: issues: read is sufficient.This workflow reads metrics and posts to Discord; it doesn’t write issues. Least‑privilege reduces blast radius.
Apply this diff:
permissions: contents: read - issues: write + issues: read pull-requests: read
47-56: Optional: add a lightweight diagnostics step before parsing.Helps troubleshoot empty/absent files without failing the job. No strict‑mode changes.
Apply this diff:
- name: Generate PR metrics uses: github/issue-metrics@v3 env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} SEARCH_QUERY: "repo:${{ github.repository }} is:pr created:${{ env.first_day }}..${{ env.last_day }}" OUTPUT_FILE: pr_metrics.md + + - name: Debug generated metrics + run: | + echo "Listing markdown files in workspace:" + ls -la *.md || true + for f in issue_metrics.md pr_metrics.md; do + if [ -f "$f" ]; then + echo "== $f (first 20 lines) ==" + head -n 20 "$f" + else + echo "Missing $f" + fi + done
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/parse-metrics.mjs(1 hunks).github/workflows/weekly-metrics-discord.yml(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
📚 Learning: 2025-09-09T19:14:21.367Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1194
File: .github/workflows/ci.yml:21-21
Timestamp: 2025-09-09T19:14:21.367Z
Learning: User Crunchyman-ralph considers suggestions about adding safer default shell options (like bash -euo pipefail) to GitHub Actions workflows as too nitpicky and prefers not to implement them.
Applied to files:
.github/workflows/weekly-metrics-discord.yml
🪛 GitHub Actions: CI
.github/scripts/parse-metrics.mjs
[error] 1-1: Formatter would have printed changes for parse-metrics.mjs. Run 'biome format .' (or the project’s formatter) to apply formatting fixes.
🔇 Additional comments (10)
.github/workflows/weekly-metrics-discord.yml (4)
20-27: Good: repo checkout and Node 20 setup early in the job.This ensures the script and dependencies are available when parsing metrics.
39-46: Don’t hide response‑time rows you intend to display. Also pin the issue output filename.The Discord card shows “Avg Response Time” for issues, but HIDE_TIME_TO_ANSWER=true removes that row from issue_metrics.md, yielding N/A. Also, the parser expects issue_metrics.md; pin the output to avoid coupling to action defaults.
Apply this diff:
- name: Generate issue metrics uses: github/issue-metrics@v3 env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} SEARCH_QUERY: "repo:${{ github.repository }} is:issue created:${{ env.first_day }}..${{ env.last_day }}" - HIDE_TIME_TO_ANSWER: true + HIDE_TIME_TO_ANSWER: false HIDE_LABEL_METRICS: false + OUTPUT_FILE: issue_metrics.md#!/bin/bash # Confirm the “Time to first response” row appears in issue_metrics.md after the change rg -nP '^\|\s*Time to first response\s*\|' issue_metrics.md -C1 || echo "Row missing"
65-79: Align labels with parsed PR stats (“merged” vs “closed”).Discord labels say “Merged”/“Avg Time to Merge”, but the parser currently uses “closed”/“Time to close”. Fix parser (see script comments) or rename labels to “Closed”. Prefer fixing parser.
30-38: Fix 14‑day window off‑by‑one and avoid partial “today” data.Anchor on yesterday and compute a true 14‑day inclusive range ending yesterday.
File: .github/workflows/weekly-metrics-discord.yml Lines: 30-38
Apply this diff:
- # Last 14 days - first_day=$(date -d "14 days ago" +%Y-%m-%d) - last_day=$(date +%Y-%m-%d) + # Last 14 full days (inclusive), ending yesterday + last_day=$(date -d "yesterday" +%Y-%m-%d) + first_day=$(date -d "$last_day -13 days" +%Y-%m-%d) @@ - echo "date_range=Past 14 days ($first_day to $last_day)" >> $GITHUB_ENV + echo "date_range=Past 14 days ($first_day to $last_day)" >> $GITHUB_ENVValidate with this self-contained script (does not rely on pre-set env vars):
#!/bin/bash last_day=$(date -d "yesterday" +%Y-%m-%d) first_day=$(date -d "$last_day -13 days" +%Y-%m-%d) echo "first_day=$first_day last_day=$last_day" start=$(date -d "$first_day" +%s) end=$(date -d "$last_day" +%s) echo $(( (end-start)/86400 + 1 )) # expect 14.github/scripts/parse-metrics.mjs (6)
3-5: Consolidate fs imports.Single import is cleaner.
Apply this diff:
-import { readFileSync, existsSync } from 'fs'; -import { writeFileSync } from 'fs'; +import { readFileSync, existsSync, writeFileSync } from 'fs';
6-20: Harden table parsing; avoid brittle index assumptions.Match a markdown row for the metric name and capture the first value cell.
Apply this diff:
function parseMetricsTable(content, metricName) { const lines = content.split('\n'); for (let i = 0; i < lines.length; i++) { const line = lines[i].trim(); - if (line.includes(metricName)) { - // Split by | and get the value column (usually index 2) - const columns = line.split('|').map(col => col.trim()); - if (columns.length >= 3) { - return columns[2] || 'N/A'; - } - } + // Match a row like: | Metric Name | value | ... + const safeName = metricName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const re = new RegExp(`^\\|\\s*${safeName}\\s*\\|\\s*([^|]+)\\|?`); + const m = line.match(re); + if (m) return m[1].trim() || 'N/A'; } return 'N/A'; }
22-27: Make numeric extraction robust (commas/units).Current parseInt breaks on “1,234” or “123 items”.
Apply this diff:
function parseCountMetric(content, metricName) { const result = parseMetricsTable(content, metricName); - // Try to extract just the number if it's a valid number - const number = parseInt(result); - return isNaN(number) ? 0 : number; + const match = String(result).replace(/,/g, '').match(/\d+/); + const number = match ? parseInt(match[0], 10) : NaN; + return Number.isNaN(number) ? 0 : number; }
51-59: Fix PR “merged vs closed” mismatch and add safe fallbacks.Labels in the workflow use “Merged/Merge”. Prefer merge metrics; fall back to close if absent.
Apply this diff:
// Parse PR metrics if (existsSync('pr_metrics.md')) { const prContent = readFileSync('pr_metrics.md', 'utf8'); metrics.prs_created = parseCountMetric(prContent, 'Total number of items created'); - metrics.prs_merged = parseCountMetric(prContent, 'Number of items closed'); - metrics.pr_avg_first_response = parseMetricsTable(prContent, 'Time to first response'); - metrics.pr_avg_merge_time = parseMetricsTable(prContent, 'Time to close'); + const mergedCount = parseCountMetric(prContent, 'Number of items merged'); + metrics.prs_merged = mergedCount || parseCountMetric(prContent, 'Number of items closed'); + metrics.pr_avg_first_response = parseMetricsTable(prContent, 'Time to first response'); + const maybeMergeTime = parseMetricsTable(prContent, 'Average time to merge'); + metrics.pr_avg_merge_time = + maybeMergeTime !== 'N/A' ? maybeMergeTime : parseMetricsTable(prContent, 'Time to close'); }
61-69: Also print final metrics to stdout.Useful locally and in CI logs.
Apply this diff:
// Output for GitHub Actions const output = Object.entries(metrics) .map(([key, value]) => `${key}=${value}`) .join('\n'); // Write to GITHUB_OUTPUT if in GitHub Actions if (process.env.GITHUB_OUTPUT) { - writeFileSync(process.env.GITHUB_OUTPUT, output + '\n', { flag: 'a' }); + try { + writeFileSync(process.env.GITHUB_OUTPUT, output + '\n', { flag: 'a' }); + } catch (error) { + console.error('Failed to write to GITHUB_OUTPUT:', error.message); + process.exit(1); + } } + console.log('\n=== FINAL METRICS ==='); + console.log(output);
1-1: Run Biome formatter and commit formatting changes
Executenpx biome format .to align.github/scripts/parse-metrics.mjswith the project style.
- Prefer 'Number of items merged' over 'Number of items closed' for accuracy - Prefer 'Average time to merge' over 'Time to close' for PR timing - Add safe fallbacks if action doesn't provide merged-specific metrics - Fixes Discord report showing 'merged' but parsing 'closed' data
- Add separate steps for PR created metrics and PR merged metrics - Query PRs by merge date for more accurate merge timing data - Parse both pr_created_metrics.md and pr_merged_metrics.md files - Maintain backward compatibility with fallback to old pr_metrics.md - Fixes timing discrepancy where merge time was faster than response time
- Rename step from 'Get dates for last week' to 'Get dates for last 14 days' - Add consistent warning messages when metrics files are missing - Use console.warn for missing file scenarios with [parse-metrics] prefix - Improve debugging visibility for fallback behavior
- Replace brittle column index parsing with regex matching - Escape special characters in metric names for safe regex - More reliable parsing against column drift and table format variations - Prevents false matches from substring matching
- Change issues permission from write to read (least privilege) - Add debug step to show generated metrics files before parsing - Helps troubleshoot empty/missing files without failing the job - Updated file list to match current pr_created_metrics.md structure
1 participant
What type of PR is this?
Description
Related Issues
How to Test This
# Example commands or stepsExpected result:
Contributor Checklist
npm run changesetnpm testnpm run format-check(ornpm run formatto fix)Changelog Entry
For Maintainers
Summary by CodeRabbit
New Features
Bug Fixes
Style