chore: add required measurable impact verification to issue and PR templates#35
chore: add required measurable impact verification to issue and PR templates#35venikman wants to merge 2 commits into
Conversation
…mplates Co-Authored-By: Stanislau <nedbailov375426@gmail.com>
Original prompt from Stanislau
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughFour GitHub template files were updated to add a required "Measurable impact verification" field/section prompting contributors to provide quantifiable before/after metrics and explicit measurement instructions for bug reports, feature requests, tasks, and pull requests. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds a required “Measurable impact verification” section/field to GitHub PR and issue templates so changes are accompanied by quantifiable before/after metrics and runnable measurement commands.
Changes:
- Add a “Measurable impact verification” section to the PR template.
- Add a required “Measurable impact verification” textarea to all three issue templates (bug/feature/task).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/pull_request_template.md | Adds a required impact-metrics section with a before/after measurement table. |
| .github/ISSUE_TEMPLATE/task.yml | Adds a required impact verification textarea with a metrics table placeholder. |
| .github/ISSUE_TEMPLATE/feature_request.yml | Adds a required impact verification textarea with a metrics table placeholder. |
| .github/ISSUE_TEMPLATE/bug_report.yml | Adds a required impact verification textarea with a metrics table placeholder (broken vs target). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | New MCP tools available | 0 | 2 | `grep -c createTool src/mcp/tools.ts` | | ||
| | Test coverage for feature | 0 tests | ≥5 tests | `bun run test` | |
There was a problem hiding this comment.
The placeholder example “New MCP tools available | 0 | 2 | grep -c createTool src/mcp/tools.ts” is internally inconsistent: grep -c createTool measures total createTool calls (currently non-zero), not “new tools”, so the suggested Before/After values won’t match the measurement command. Consider making the example metric match the command (e.g., total tool count), or change the command to count specific new tool IDs via grep -Ec on those IDs.
| | New MCP tools available | 0 | 2 | `grep -c createTool src/mcp/tools.ts` | | |
| | Test coverage for feature | 0 tests | ≥5 tests | `bun run test` | | |
| | Total MCP tool definitions in `src/mcp/tools.ts` | current count | current count + 2 | `grep -c createTool src/mcp/tools.ts` | | |
| | Feature test cases added | 0 | ≥5 | `grep -Ec '\b(it|test)\(' path/to/feature.test.ts` | |
There was a problem hiding this comment.
Good catch — the placeholder example was inconsistent: grep -c createTool counts total definitions, not "new" ones. Fixed: now shows Total MCP tool definitions | 10 | 12 which matches the measurement command, and the second example uses a targeted grep -Ec for test cases.
…lot review Co-Authored-By: Stanislau <nedbailov375426@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a mandatory 'Measurable impact verification' section across the bug report, feature request, and task issue templates, as well as the pull request template. This section requires users to provide quantifiable metrics in a table format to verify the impact of their changes. A review comment suggested updating a file path in the feature request example to maintain consistency with the project's established directory structure.
| placeholder: | | ||
| | Metric | Before | After (target) | How to measure | | ||
| |--------|--------|----------------|----------------| | ||
| | Total MCP tool definitions | 10 | 12 | `grep -c createTool src/mcp/tools.ts` | |
There was a problem hiding this comment.
The example command references src/mcp/tools.ts, but the project's boundary checks (as defined in the PR template, line 45) specify that MCP tool contracts should reside in src/mcp/tool-contracts.ts. Updating the example to use the correct file path maintains consistency with the project's established structure.
| New MCP tools available | 0 | 2 | `grep -c createTool src/mcp/tool-contracts.ts` |There was a problem hiding this comment.
This is a false positive. The boundary check says "MCP tool contracts stay in src/mcp/tool-contracts.ts" — meaning Zod input/output schemas. The actual createTool calls live in src/mcp/tools.ts:
$ grep -c createTool src/mcp/tools.ts
10
$ grep -c createTool src/mcp/tool-contracts.ts
0
The example correctly references tools.ts for counting tool definitions.
Code ReviewOverviewAdds a required Strengths
IssuesSupersedes #34#34 is the earlier rev of the same change. Close #34 when this merges. PR template is honor-systemThe PR template (markdown, not yaml form) can't enforce
Friction on trivial PRsSame concern as noted in my #34 review: typo fixes, docs-only changes, and pure chores don't have meaningful before/after metrics. Consider:
Table shape is a bit heavy4 columns per row (Metric / Before / After / How to measure) is correct but a single metric can end up wrapping oddly in narrow GitHub displays. Not a blocker. Example metric choices are good
Bootstrap questionDoes this PR itself include a measurable-impact table in the body? (I didn't see one in RecommendationApprove, close #34 as superseded, and follow up with the CI enforcement so the PR-template field isn't purely honor-system. The quality of the rollout (consistent labels, tight examples, explicit guardrail) is noticeably better than #34. |
|
Thanks for the review. A few notes:
All bot review comments (Copilot, Gemini) were already addressed in earlier commits. CI green (2/2). This looks merge-ready to me — shall I merge it? |
|
Closing — PR #34 (earlier version with similar changes) was already closed without merging. Cleaning up as part of repo housekeeping. |
1 similar comment
|
Closing — PR #34 (earlier version with similar changes) was already closed without merging. Cleaning up as part of repo housekeeping. |
What
Adds a required "Measurable impact verification" field to all three issue templates (bug report, feature request, task) and adds a corresponding section to the PR template. Also fixes incorrect measurement commands in 4 existing open issues (#2, #4, #5, #15).
Why
Every issue and PR should have quantifiable, verifiable impact metrics — no vague criteria like "works better" or "improved". This enforces a before/after table with concrete metrics and runnable measurement commands.
Type
feat— new capabilityfix— bug fixrefactor— code restructuringdocs— documentation onlychore— maintenance (deps, CI, cleanup)Changes
bug_report.yml: Added requiredMeasurable impact verificationtextarea with before/after table placeholderfeature_request.yml: Added requiredMeasurable impact verificationtextarea with before/after table placeholdertask.yml: Added requiredMeasurable impact verificationtextarea (placed before Agent-delegable dropdown)pull_request_template.md: AddedMeasurable impact verificationsection between Changes and ValidationIssue body fixes (via
gh issue edit):'C.17'), not double quotes; old commands returned 0 false negativesrelationGraph.filtergrep (code usesfor...of, not.filter())grep -Ec 'browse|search'(matchedsearchableTextfalse positive) → now uses exact schema namesMeasurable impact verification
grep -c 'required: true' .github/ISSUE_TEMPLATE/*.yml(impact fields only)grep -c 'Measurable impact verification' .github/pull_request_template.mdgrep -Ec "'C\.[0-9]|'A\.[0-9]|'B\.[0-9]|'F\.[0-9]" src/runtime/query-engine.tsgrep -c 'for.*of.*\.relationGraph' src/runtime/query-engine.tsValidation
bun run lintpasses locallybun run checkpasses locallybun run testpasses locallybun run buildsucceeds (if runtime/server code touched)bun run docs:buildsucceeds (if docs touched)Boundary check
FPF-spec.mdonly — no additional corpora addedsrc/mcp/tool-contracts.tsAgent metadata
Requested by: @venikman
Summary by CodeRabbit
Chores
Documentation