docs: set an explicit contribution bar for agent-authored PRs - #2602
Conversation
A growing share of incoming PRs assert engine behaviour that turns out to be false when tested, guard states that cannot occur, or carry `fix:` for changes with no observable effect. Reviewing them costs more than the changes are worth, and nothing in the repo told contributors where the bar is. State it: reproduce before claiming, label honestly, make tests exercise the code they cover and verify they actually run in CI, one change per PR, and rebase before review. The wren package doc gains the specific fact behind the most common case — manifest schema is validated once in wren-core, so per-callsite guards downstream are dead code, while real input boundaries should validate and report rather than crash. The PR template asks the same questions at the point of authoring, since not every contributor reads CLAUDE.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
documentation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughRepository guidance adds contribution standards, instruction pointers, a pull request template, and documentation describing the MDL manifest validation boundary. ChangesRepository guidance
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/pull_request_template.md (1)
6-38: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd author checks for every mandatory Contribution Bar gate.
The template prompts for reproduction, tests, and duplicate checks, but not honest PR labeling, documented-decision changes, or rebasing. Add explicit checkboxes (or equivalent fields) so authors attest to all requirements from
.claude/CLAUDE.mdLines 136-169.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/pull_request_template.md around lines 6 - 38, Add explicit author-attestation fields to the pull request template covering honest PR labeling, documented-decision changes, and rebasing, alongside the existing failure, testing, and duplicate checks. Align the new requirements with the mandatory Contribution Bar gates defined in CLAUDE.md, while preserving the current prompts and template structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/wren/.claude/CLAUDE.md`:
- Line 49: Revise the “Validation boundaries — validate once, at the edge”
guidance to acknowledge that context.py decodes the manifest and accesses
manifest["views"] before WrenEngine construction, so malformed raw JSON/base64
input can reach Python unvalidated. State that wren-core remains the
schema-validation authority, but raw input must be structurally validated before
Python dereferences it; retain the prohibition on redundant guards only for
post-deserialization paths.
---
Outside diff comments:
In @.github/pull_request_template.md:
- Around line 6-38: Add explicit author-attestation fields to the pull request
template covering honest PR labeling, documented-decision changes, and rebasing,
alongside the existing failure, testing, and duplicate checks. Align the new
requirements with the mandatory Contribution Bar gates defined in CLAUDE.md,
while preserving the current prompts and template structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 70aad793-baa5-4db3-91b6-34973eb998f5
📒 Files selected for processing (3)
.claude/CLAUDE.md.github/pull_request_template.mdcore/wren/.claude/CLAUDE.md
The per-module crates already carry this pointer file; the repository root did not, so agents that read AGENTS.md rather than CLAUDE.md found nothing at the top level and never reached the contribution bar. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 3-7: Revise the repository-wide guidance statement in AGENTS.md to
clarify that the root .claude/CLAUDE.md contains repository-wide instructions,
while applicable module-specific guidance must also be read from
core/<module>/.claude/CLAUDE.md. Preserve the existing direction to consult the
Contribution Bar and the per-module AGENTS.md references.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
context.validate_manifest decodes the manifest and iterates manifest["views"] before WrenEngine is constructed, and construction does not deserialize — so Python can and does observe an unvalidated dict. The guarantee only holds downstream of a successful wren-core deserialization; say so, and name that function as the counter-example. Also narrow the root AGENTS.md opening line, which claimed all instructions live in .claude/CLAUDE.md while the same file points at per-module ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Pushed Validation-boundary timing claim — correct, thanks. Root Attestation checkboxes in the PR template — declining, deliberately. Three reasons:
The Contribution Bar in |
validate_manifest was the wrong illustration. It does dereference a decoded manifest before wren-core sees it, but its only caller rebuilds the manifest via build_json(), so malformed input cannot actually reach it — citing it as a "genuine input boundary" would invite exactly the unreachable-state guard the paragraph above forbids. convert_mdl_to_project is the real case: `wren context init --from-mdl` reads a user-supplied JSON file and passes it straight in, with nothing in between. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
Summary
Adds a
## Contribution Barsection to the rootCLAUDE.md, one design-point entry to thewrenpackageCLAUDE.md, and a pull request template.What problem does this solve?
Review load, not a code defect. A growing share of incoming PRs share a few recurring shapes:
isinstancechecks over manifest structures thatwren-core's serde layer has already rejected a few lines earlier.fix:on changes with no observable effect, which makes the changelog claim repairs that did not happen.mainfor about a day, because its last CI run predated a refactor that renamed the helper it called. See CI: stale required checks let a semantic merge conflict (F821) reach main #2598.None of this was written down anywhere, so there was no shared standard to point at in review.
What's in it
.claude/CLAUDE.md— aContribution Barsection: prove the problem before fixing it, label honestly, make tests exercise the changed code and confirm they run in CI, one change per PR, don't silently reverse documented decisions, rebase before review. Each rule is written to be self-checkable before opening a PR and citable by name in review.core/wren/.claude/CLAUDE.md— a "Validation boundaries" design point recording where MDL manifest schema is actually enforced (wren-coreserde, viaManifestExtractor/SessionContext), and the contrast with genuine input boundaries — raw user JSON, hand-edited project YAML, external API responses — where validating and reporting is the right behaviour.AGENTS.md(new, repo root) — a pointer to.claude/CLAUDE.md, matching the existing per-moduleAGENTS.mdfiles, so agents that look forAGENTS.mdrather thanCLAUDE.mdreach the contribution bar. The root was the one level that lacked this file..github/pull_request_template.md— the same questions at authoring time, for contributors who don't readCLAUDE.md. Explicitly skippable for typo and docs changes.Notes
Deliberately describes patterns rather than naming contributors or citing the PRs that prompted it; this file stays in the repo long after any individual PR is closed. The two incident references above (#2482, #2598) are process history rather than attribution.
Docs only — no code or CI changes.
🤖 Generated with Claude Code
Summary by CodeRabbit