Repository navigation
docs: add first contributor fast paths - #692
flyingrobots merged 12 commits into
Conversation
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds contributor onboarding documentation (a "First PR" guide) and a security-tooling policy document, linking both from CONTRIBUTING.md, SECURITY.md, and various docs topic pages. It also updates SDL lowering in apollo.rs to enforce repeatable-vs-non-repeatable custom directive semantics, with new tests. ChangesRepeatable Directive Lowering
Estimated code review effort: 4 (Complex) | ~55 minutes Contributor Onboarding and Security-Tooling Docs
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cf3d1aa4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…sture docs: document security tooling posture
…ive-collisions fix(core): reject duplicate nonrepeatable directives
🔍 The Case of Pull Request #692Schema Sets
Schema Set
|
Self-Code Review Findings@codex please sanity-check this self-discovered issue.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/wesley-core/src/adapters/apollo.rs (1)
602-613: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInconsistent/duplicate error path for core-directive duplicates.
The early check at Lines 602-607 rejects a duplicate core directive with message
"Duplicate directive '@{name}'". However, sincerepeatableis forced tofalsefor core directives (core_name.is_none() && ...at Line 609),insert_directive_value(Lines 1149-1173) would independently reject the same duplicate with a different message:"Duplicate non-repeatable directive '@{name}'". The Lines 602-607 check is therefore dead in practice except for producing a differently-worded error for the same failure mode as custom non-repeatable directives, which is confusing for consumers matching on diagnostic messages.♻️ Proposed fix: remove the redundant early check and let insert_directive_value handle it uniformly
- if core_name.is_some() && map.contains_key(&canonical_name) { - return Err(lowering_error_value( - "directive", - format!("Duplicate directive '@{canonical_name}'"), - )); - } - let repeatable = core_name.is_none() && repeatable_directives.contains(&canonical_name); insert_directive_value(map, canonical_name, val, repeatable)?;Also applies to: 1149-1173
🤖 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 `@crates/wesley-core/src/adapters/apollo.rs` around lines 602 - 613, The core-directive duplicate check in apollo.rs is redundant and creates a second error path with a different message than insert_directive_value. Remove the early map.contains_key duplicate return in the directive-lowering logic and rely on insert_directive_value to reject all non-repeatable duplicates uniformly; this keeps core directives and custom directives consistent via the shared repeatable handling around repeatable_directives and insert_directive_value.
🧹 Nitpick comments (1)
crates/wesley-core/src/adapters/apollo.rs (1)
401-476: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepetitive directive-extraction boilerplate across type kinds.
merge_definitionandmerge_extensioneach repeat the sameif let Some(dirs) = node.directives() { Self::extract_directives_with_repeatability(dirs, &mut acc.directives, repeatable_directives)?; }pattern for everyTypeDefinitionNode/TypeExtensionNodevariant (6 variants each, 12 total occurrences). Consider factoring this into a small helper takingOption<cst::Directives>to reduce duplication and ease future changes to directive handling.Also applies to: 489-564
🤖 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 `@crates/wesley-core/src/adapters/apollo.rs` around lines 401 - 476, The directive extraction logic in merge_definition and merge_extension is duplicated across every TypeDefinitionNode and TypeExtensionNode branch. Factor the repeated if let Some(dirs) = node.directives() call pattern into a small helper that accepts the optional directives and applies extract_directives_with_repeatability, then reuse it in each variant to keep directive handling centralized and reduce boilerplate.
🤖 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.
Outside diff comments:
In `@crates/wesley-core/src/adapters/apollo.rs`:
- Around line 602-613: The core-directive duplicate check in apollo.rs is
redundant and creates a second error path with a different message than
insert_directive_value. Remove the early map.contains_key duplicate return in
the directive-lowering logic and rely on insert_directive_value to reject all
non-repeatable duplicates uniformly; this keeps core directives and custom
directives consistent via the shared repeatable handling around
repeatable_directives and insert_directive_value.
---
Nitpick comments:
In `@crates/wesley-core/src/adapters/apollo.rs`:
- Around line 401-476: The directive extraction logic in merge_definition and
merge_extension is duplicated across every TypeDefinitionNode and
TypeExtensionNode branch. Factor the repeated if let Some(dirs) =
node.directives() call pattern into a small helper that accepts the optional
directives and applies extract_directives_with_repeatability, then reuse it in
each variant to keep directive handling centralized and reduce boilerplate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b7c90e7a-3c8b-45be-8811-e5a6b1bec8bd
📒 Files selected for processing (13)
CHANGELOG.mdCONTRIBUTING.mdSECURITY.mdcrates/wesley-core/src/adapters/apollo.rscrates/wesley-core/tests/lowering_validation.rsdocs/reference/directives.mddocs/topics/README.mddocs/topics/ci-workflows.mddocs/topics/contributing/first-pr.mddocs/topics/docs-orientation.mddocs/topics/security-tooling.mddocs/topics/validation.mddocs/truth-manifest.json
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b484afccd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Activity Summary
Validation:
Merge gate: clean and green for admin merge. |
53ba7f7
into
evidence/rust-ir-performance-baseline
1 participant
Summary
docs/topics/contributing/first-pr.mdwith a practical first-PR path, required-vs-optional reading, starter issue standards, fast lanes, and exact maintainer triage/release commands.CONTRIBUTING.md, the topic map, docs orientation, and the docs truth manifest.Validation
pnpm exec prettier --write CHANGELOG.md CONTRIBUTING.md docs/topics/README.md docs/topics/docs-orientation.md docs/topics/contributing/first-pr.md docs/truth-manifest.jsoncargo xtask docs-checkpnpm run preflightgit diff --checkCloses #682
Summary by CodeRabbit
New Features
Bug Fixes
Documentation