RFC: Contribution Quality & Process Overhaul

[joshuadarron]

Context

RocketRide has grown fast. Stars, forks, and contributors have scaled significantly in a short time, and that's a good thing. But our contribution process hasn't kept pace. Right now we have:

• PRs merging without human review
• Incomplete checklists being ignored
• Broken code reaching develop
• No granular CI signal (one failing check buries everything else)

These are problems worth solving before the contributor volume increases further. What follows is how I'm thinking about it. I'd love to hear where people agree, disagree, or have better ideas.

---

1. Branch Protection Rules

My thinking is that merging into develop should require:

• At least 2 human approvals, with at least 1 from a CODEOWNER for the modified paths. CodeRabbit shouldn't count toward either.
• All CI checks passing.
• No self-merging, regardless of write access.
• Stale review dismissal on new commits. A force-push or new commit after approval would require re-approval.

---

2. Granular CI Checks

Right now CI is effectively a black box. A single failing Python test buries the TypeScript lint result and nobody can triage at a glance. I think splitting CI into discrete checks would help a lot:

Check	Scope	Blocks merge?
ci/cpp-tests	C++ unit tests via CTest	Yes
ci/python-nodes-tests	pytest across nodes/ (smoke, module, security suites)	Yes
ci/python-sdk-tests	pytest for packages/client-python/	Yes
ci/ts-chat-ui	Vitest for apps/chat-ui/	Yes
ci/ts-dropper-ui	Vitest for apps/dropper-ui/	Yes
ci/ts-sdk	Vitest for packages/client-typescript/ and packages/client-mcp/	Yes (once tests exist)
ci/lint-format	ESLint + Prettier (TS), Ruff (Python), clang-format (C++)	Yes
ci/type-check	tsc --noEmit + pyright	Yes
ci/build-verify	Full build completes without error	Yes
ci/docstring-coverage	Python docstring coverage (threshold: see below)	Warning now, blocker phased in
ci/security-scan	Bandit (Python), CodeQL (TS/C++)	Warning for now

The tests already run individually so this is primarily a wiring task: splitting each into its own GitHub Actions job rather than running everything in one action.

On docstring coverage: PR #464 is sitting at 15.79% against an 80% threshold. Flipping that to a hard block overnight would close half our open PRs. My thought is to set the block threshold at 40% now, publish a roadmap to 80%, and ratchet it up quarterly. Any new code added in a PR would need to meet 80% on the delta immediately, not the whole codebase.

On ci/ts-sdk: tests for packages/client-typescript/ and packages/client-mcp/ don't exist yet. This check would be added once they do. Worth a separate conversation about what coverage looks like for the SDK public API.

---

3. PR Template & Branch Naming

A consistent branch naming format would make PRs easier to triage and enable auto-linking to issues:

{type}/RR-{issue-number}-{brief-description}

Valid types: feat, fix, refactor, docs, test, chore, ci

Examples:

feat/RR-429-exa-search-node
fix/RR-337-ssrf-http-request
refactor/RR-464-native-cpp-chunker
docs/RR-211-python-sdk-readme

A GitHub Action could validate the format on PR open and label needs: branch rename if it doesn't match.

For the PR template itself, something like:

## What changed
[Brief description of what this PR does. One paragraph max.]
 
## Why
[Explain the motivation. Reference a related issue if one exists.]
 
## How to test
[Specific steps a reviewer can follow to verify this works. Describe what to do and what to look for.]
 
## Checklist
- [ ] Branch follows naming convention
- [ ] Tests added or updated for all changed behavior
- [ ] All existing tests pass locally
- [ ] Public APIs and functions have docstrings
- [ ] Docs updated if behavior changed
- [ ] No unrelated changes in this PR
- [ ] I have reviewed my own diff before requesting review

If a checklist item doesn't apply, a comment explaining why is fine. Reviewers can use their judgment, but an unexplained blank checklist is going to slow things down.

---

4. Stale PR Policy

PRs that go quiet create drag for everyone. My thinking:

• 14 days without contributor engagement (no commits, no comment responses): PR gets labeled stale
• Stale PRs wouldn't be auto-closed. The label is a signal, not a sentence.
• Maintainers could reach out or close at their discretion if a stale PR is blocking related work.
• Any commit or comment from the contributor removes the label.

---

5. Comment Resolution Policy

A few norms I think would help keep reviews clean:

• The reviewer resolves their own comments, not the author. Clicking "resolve" means "I've verified this is addressed."
• Authors respond to all open comments before requesting re-review.
• Pushback is welcome. A clear disagreement with an explanation is a productive review. An unacknowledged comment isn't.
• All conversations resolved before merge.

---

6. RFC Process for Non-Trivial Changes

For changes that have broad impact, I think it's worth having a lightweight RFC process before code gets written. My proposed triggers:

• Modifies the public pipeline JSON schema
• Changes the WebSocket protocol or message format
• Modifies a public SDK API (TypeScript, Python, or MCP)
• Adds or removes a node type
• Makes architectural changes to the C++ core
• Introduces a new external dependency to the engine or core libraries
• Adds a new CI/CD pipeline or changes how the build system works

The process would be simple: open a GitHub Discussion with the RFC label, include a problem statement, proposed solution, alternatives considered, and tradeoffs. Tag relevant CODEOWNERS. Give it at least 3 business days before starting implementation. The implementation PR links back to the discussion.

If you're unsure whether something qualifies, opening a Discussion and asking takes 5 minutes.

---

7. CONTRIBUTING.md

The current CONTRIBUTING.md is the default 5-step boilerplate. Whatever we agree on here should be reflected there so it's the single source of truth for contributors.

---

Curious what the team thinks. Where does this make sense, where does it not, and what's missing?