docs: contributor onboarding path + CODEOWNERS for bus-factor mitigation#132
Closed
clean6378-max-it wants to merge 1 commit into
Closed
docs: contributor onboarding path + CODEOWNERS for bus-factor mitigation#132clean6378-max-it wants to merge 1 commit into
clean6378-max-it wants to merge 1 commit into
Conversation
Add docs/onboarding.md with first-PR walkthrough, reading order, and full local gate commands. Wire .github/CODEOWNERS for review routing and record bus-factor risk + mitigation in README with cross-links from CONTRIBUTING and architecture docs. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughAdds a contributor onboarding guide, links it from existing documentation, documents maintainer coverage, and introduces CODEOWNERS rules for repository, documentation, and GitHub configuration paths. ChangesContributor process
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/CODEOWNERS (1)
7-16: 📐 Maintainability & Code Quality | 🔵 TrivialAdd backup ownership if this is intended to mitigate bus factor.
Every rule routes to
@clean6378-max-it, so this provides deterministic routing but no reviewer redundancy. Consider adding a second maintainer/team owner or documenting an escalation path.🤖 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/CODEOWNERS around lines 7 - 16, Add backup ownership to the CODEOWNERS rules by assigning a second maintainer or team alongside `@clean6378-max-it`, or document an explicit escalation path if backup ownership is handled elsewhere. Apply the chosen redundancy consistently to the default, documentation, and CI/workflow rules.
🤖 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 `@docs/onboarding.md`:
- Around line 63-87: Update the “Run the full local gate” section to match CI:
state that only applicable checks run on each platform rather than claiming all
commands run on Ubuntu, Windows, and macOS; replace the mypy command with the
production package checks for api, utils, and models plus the separate tests
check using mypy-tests.ini and follow-imports skip; and include
tests/test_search.py alongside tests/test_api_integration.py in the integration
command.
---
Nitpick comments:
In @.github/CODEOWNERS:
- Around line 7-16: Add backup ownership to the CODEOWNERS rules by assigning a
second maintainer or team alongside `@clean6378-max-it`, or document an explicit
escalation path if backup ownership is handled elsewhere. Apply the chosen
redundancy consistently to the default, documentation, and CI/workflow rules.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 45d96ac3-075d-4250-b0bf-ba98c6f08e5b
📒 Files selected for processing (5)
.github/CODEOWNERSCONTRIBUTING.mdREADME.mddocs/architecture.mddocs/onboarding.md
Comment on lines
+63
to
+87
| ### 5. Run the full local gate | ||
|
|
||
| CI runs these on Ubuntu, Windows, and macOS. Run them locally before opening a PR: | ||
|
|
||
| ```bash | ||
| # Lint + format (required) | ||
| ruff check . | ||
| ruff format --check . | ||
|
|
||
| # Type check (Ubuntu CI job; run locally before Python-heavy changes) | ||
| mypy -p api -p utils -p models -p scripts | ||
|
|
||
| # Security audit (production deps) | ||
| pip-audit -r requirements.txt | ||
|
|
||
| # Python tests (full suite) | ||
| pytest -q | ||
|
|
||
| # Integration subset (also run in CI) | ||
| pytest tests/test_api_integration.py -v | ||
|
|
||
| # Frontend — only if you changed static/js/ | ||
| npm ci | ||
| npm test | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align the documented local gate with CI.
This section currently differs from the supplied CI contract:
- CI runs
mypy -p api -p utils -p models, then separately checks tests withmypy tests --config-file mypy-tests.ini --follow-imports skip; it does not runmypy -p scripts. - The integration job runs both
tests/test_api_integration.pyandtests/test_search.py, but this guide omits the search tests. - Mypy is documented as an Ubuntu-only CI check elsewhere, so “these” should not claim every command runs on all three operating systems.
Please update the commands and platform wording so contributors can reproduce the actual gate.
🤖 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 `@docs/onboarding.md` around lines 63 - 87, Update the “Run the full local
gate” section to match CI: state that only applicable checks run on each
platform rather than claiming all commands run on Ubuntu, Windows, and macOS;
replace the mypy command with the production package checks for api, utils, and
models plus the separate tests check using mypy-tests.ini and follow-imports
skip; and include tests/test_search.py alongside tests/test_api_integration.py
in the integration command.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
docs/onboarding.mdwith first-PR walkthrough (fork → branch → dev setup → full local gate → PR), suggested reading order (architecture → jsonl_parser → dispatch → SPA), good-first-issue links, and bus-factor mitigation context..github/CODEOWNERSnaming @clean6378-max-it with documented review expectation (no self-merge without approval).CONTRIBUTING.md,README.md(brief bus-factor note), anddocs/architecture.md.Docs/process only — no source-behavior change.
Fixes #129
Test plan
ruff check .green (docs-only; no Python source changes)docs/onboarding.md,CONTRIBUTING.md, andREADME.mdresolve.github/CODEOWNERSsyntax follows GitHub code owners formatMade with Cursor
Summary by CodeRabbit