feat(intelligence): GitHub 連携スキル推論基盤 Phase1+2(ADR-0016)#408
Conversation
ADR-0016 に基づき、GitHub 連携のスキル推論を 3 層モデル (Skill / Evidence / Proficiency)へ再設計する基盤と、discover(Linguist resolve)+ declare(manifest 宣言)までを実装する。verify / monorepo / private / deps.dev は ADR どおり後続フェーズ。 - discover: Linguist 内部マスタ + リゾルバ(連携ホットパスで外部非依存、 data 言語の既定除外を keep_data で補正 / D3・D4) - declare: plugin 型 manifest パーサ(go.mod / pyproject.toml / requirements.txt / package.json / Cargo.toml)。dependency_kind を保持(D7) - 3 層モデル github_skills / github_skill_evidence / github_skill_proficiency + migration 0045(新規テーブルのみ・既存改変なし) - 集計(aggregator)→ 洗い替え永続化を github-link タスクへ配線 - 読み出し API GET /api/github-link/skills + schemas(codegen 再生成) - ADR-0016 追加 / 不要になった ADR-0011 を Deprecated 化 - テスト: parsers / linguist / aggregator / 永続化 + endpoint(認可・分離・冪等) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 50 minutes and 47 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds GitHub skill storage, language normalization, manifest parsing, aggregation, persistence, and a new skills read endpoint. It also updates generated frontend API types, adds tests for the flow, and changes one unrelated ADR status note. ChangesGitHub skill inference pipeline
Frontend textlint ADR status update
Sequence Diagram(s)sequenceDiagram
participant run_github_link
participant collect_repos
participant aggregate_skills
participant GitHubSkillRepository
run_github_link->>collect_repos: collect_manifests=True
collect_repos-->>run_github_link: RepoData list
run_github_link->>aggregate_skills: RepoSkillInput list
aggregate_skills-->>run_github_link: detected_skills
run_github_link->>GitHubSkillRepository: replace_for_user(detected_skills)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
backend/tests/test_github_skills_api.py (1)
87-97: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winIdempotency test doesn't cover orphaned evidence cleanup.
test_replace_is_idempotentonly asserts survivingcanonical_namevalues. It would not detect orphanedgithub_skill_evidencerows left behind if the Core-level delete inreplace_for_userdoesn't cascade (seebackend/app/repositories/skill.pyLines 40-42). Consider asserting the evidence row count drops to match the second replace to lock in the cascade behavior.🤖 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 `@backend/tests/test_github_skills_api.py` around lines 87 - 97, The idempotency test for GitHub skills only checks surviving canonical_name values and can miss leftover github_skill_evidence rows after replace_for_user. Update test_replace_is_idempotent in test_github_skills_api.py to also verify the evidence count after the second replace matches the remaining Python skill, so the cascade cleanup behavior in GitHubSkillRepository.replace_for_user is locked in.
🤖 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 `@backend/app/models/skill.py`:
- Around line 79-84: The raw SQL delete in
GitHubSkillRepository.replace_for_user bypasses the ORM cascades defined on
GitHubSkill relationships, so orphaned GitHubSkillEvidence and
GitHubSkillProficiency rows can remain when foreign keys are not enforced. Fix
this by either enabling PRAGMA foreign_keys=ON for every connection in
backend/app/db/database.py (so ON DELETE CASCADE works) or by changing
replace_for_user to delete through the ORM Session instead of
__table__.delete(), ensuring the evidence and proficiency relationships on
GitHubSkill are cleaned up.
In `@backend/app/repositories/skill.py`:
- Around line 40-42: The bulk delete in GitHubSkillRepository (the
self.db.execute(GitHubSkill.__table__.delete()...) path) bypasses ORM cascade,
so make sure SQLite foreign-key enforcement is enabled for the app/test engine
via the engine setup or connection hook. If that cannot be guaranteed, change
the delete flow to load the GitHubSkill entities and remove them with
session.delete so the GitHubSkill.evidence and GitHubSkill.proficiency cascades
always run regardless of backend.
In `@backend/app/services/intelligence/github_link_service.py`:
- Around line 168-171: Skill persistence is committed after the cache is already
marked completed, creating a transient inconsistent state if
GitHubSkillRepository.replace_for_user fails. In github_link_service.py, adjust
the ordering around the db.commit() call and GitHubSkillRepository(db,
user_id).replace_for_user(detected_skills) so the skill replace happens in the
same transaction before finalizing the completed status, or otherwise ensure
both writes are committed atomically. Confirm the intended transactional flow in
the surrounding service method so the cache state and persisted skills stay in
sync.
In `@backend/app/services/intelligence/skills/aggregator.py`:
- Around line 135-159: The package deduplication in `_collect_packages` is using
raw `decl.name`, which can create duplicate `DetectedSkill` entries for PyPI
names that differ only by case or separator style. Update the dedup key logic in
`_collect_packages` so that when `decl.ecosystem` is `pypi`, the package name is
PEP 503-normalized before it is stored in `best` and passed to `_upsert`; keep
other ecosystems unchanged. Use the existing `_collect_packages` and `_upsert`
flow as the place to apply the normalization consistently for canonical
matching.
In `@backend/app/services/intelligence/skills/manifests/_pep508.py`:
- Around line 9-19: extract_package_name currently misclassifies URL/VCS direct
references as package names because _NAME_RE matches the leading scheme; update
the parsing in extract_package_name to detect direct URL/VCS specs first and
return None for them, while keeping normal PEP 508 names working. Use the
existing extract_package_name helper in _pep508.py as the single place to apply
the fix so downstream callers in requirements_txt.py and pyproject.py stop
emitting invalid names.
In `@backend/tests/test_skill_linguist.py`:
- Around line 44-49: The fallback test in
test_unknown_language_falls_back_to_included currently uses Brainfuck, which may
later become a curated language and stop exercising the unknown-language path.
Update the test to use a clearly synthetic token that will never appear in
linguist_master.json, and keep the assertions on resolve_language so it verifies
the full fallback shape through the canonical result.
In `@docs/adr/0011-frontend-textlint-proofread.md`:
- Around line 5-8: The ADR index is now inconsistent because the deprecation
note for ADR-0011 no longer matches the status listed in CONTRIBUTING.md. Update
the ADR index entry for 0011 to reflect the new deprecated/rejected state when
editing this document, and verify the status text in the ADR listing stays
aligned with the deprecated notice.
---
Nitpick comments:
In `@backend/tests/test_github_skills_api.py`:
- Around line 87-97: The idempotency test for GitHub skills only checks
surviving canonical_name values and can miss leftover github_skill_evidence rows
after replace_for_user. Update test_replace_is_idempotent in
test_github_skills_api.py to also verify the evidence count after the second
replace matches the remaining Python skill, so the cascade cleanup behavior in
GitHubSkillRepository.replace_for_user is locked in.
🪄 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 Plus
Run ID: 3d4bc6b9-809f-4e3e-9c76-70179aa87bcf
📒 Files selected for processing (30)
backend/alembic_migrations/versions/0045_add_github_skill_tables.pybackend/app/models/__init__.pybackend/app/models/skill.pybackend/app/repositories/__init__.pybackend/app/repositories/skill.pybackend/app/routers/github_link.pybackend/app/schemas/github_skill.pybackend/app/services/intelligence/github/api_client.pybackend/app/services/intelligence/github_collector.pybackend/app/services/intelligence/github_link_service.pybackend/app/services/intelligence/skills/__init__.pybackend/app/services/intelligence/skills/aggregator.pybackend/app/services/intelligence/skills/linguist.pybackend/app/services/intelligence/skills/manifests/__init__.pybackend/app/services/intelligence/skills/manifests/_pep508.pybackend/app/services/intelligence/skills/manifests/base.pybackend/app/services/intelligence/skills/manifests/cargo_toml.pybackend/app/services/intelligence/skills/manifests/go_mod.pybackend/app/services/intelligence/skills/manifests/package_json.pybackend/app/services/intelligence/skills/manifests/pyproject.pybackend/app/services/intelligence/skills/manifests/registry.pybackend/app/services/intelligence/skills/manifests/requirements_txt.pybackend/app/services/intelligence/skills/types.pybackend/tests/test_github_skills_api.pybackend/tests/test_skill_aggregator.pybackend/tests/test_skill_linguist.pybackend/tests/test_skill_parsers.pydocs/adr/0011-frontend-textlint-proofread.mddocs/adr/0016-github-skill-inference.mdweb/src/api/generated.ts
.gitignore の `data/` ルールにより skills/data/linguist_master.json が コミットされず、CI のクリーンチェックアウトで FileNotFoundError になっていた (ローカルはディスク上に存在するため test が通っていた)。データファイルを skills/resources/ へ移して追跡対象にする。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- replace_for_user を ORM セッション削除に変更し、cascade="all, delete-orphan" で evidence/proficiency も確実に削除する。Core 一括 DELETE は DB の ON DELETE CASCADE に依存し、SQLite/libSQL は PRAGMA foreign_keys=ON でないと 孤児行を残すため、バックエンド非依存にする(Critical/Major 指摘)。 - github_link_service: 3 層スキルの永続化を cache を completed にする前へ移動。 永続化失敗時に「completed なのにスキル無し」を避ける(Minor 指摘)。 - idempotency テストを強化し、削除後に孤児 evidence が残らないことを assert。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- _pep508: URL/VCS 直指定(https:// / git+https:// 等)を package 名抽出前に 弾く。従来は "https"/"git" を誤って package 名として下流に流していた(Major)。 requirements.txt の回帰テストを追加。 - CONTRIBUTING.md の ADR 索引: 0011 を Deprecated に更新し、0016 を追加(索引が 本文と不整合だった指摘 / Minor)。 - linguist の未収録フォールバックテストを、将来収録され得る実在名(Brainfuck)から 合成トークンへ変更し fallback 形状を検証(Minor)。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aggregator._collect_packages の dedup キーが raw な decl.name を使っており、 pypi で大小文字・区切り違い(Flask/flask、ruamel.yaml/ruamel-yaml 等)が 別スキルとして重複していた。pypi のみ PEP 503 正規化(小文字化・-_. を - に畳む) してから canonical 名・キーに使う。go/npm/cargo は package ID をそのまま維持。 正規化と非正規化の回帰テストを追加。 なお同 PR の models/skill.py の Core DELETE 指摘は前コミットで ORM 削除に修正済み のため対象外。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
概要
ADR-0016 に基づき、GitHub 連携のスキル推論を 3 層モデル(Skill / Evidence / Proficiency) へ再設計する基盤と、discover(Linguist resolve)+ declare(manifest 宣言) までを実装します。
verify/ monorepo / private / deps.dev は ADR どおり後続フェーズです。スコープは事前に合意した Phase1+2(基盤+declare)。既存の JSON キャッシュ経路・旧
skill_taxonomyは残置し、非破壊で 3 層経路を追加しています。変更点
skills/data/linguist_master.json+skills/linguist.py。連携ホットパスで外部を叩かず resolve。エイリアス名寄せ・data 言語の既定除外(keep_dataで補正)・表示名補正(HCL→Terraform 等)。ManifestParser+ Tier1 パーサ(go.mod / pyproject.toml / requirements.txt / package.json / Cargo.toml)。dependency_kind(direct/dev/indirect/peer/build)を保持。go.modの// indirect・devDependenciesを区別。github_skills/github_skill_evidence/github_skill_proficiency+ migration0045(新規テーブルのみ・既存改変なし)。aggregatorで L1+L2 を組み立て、GitHubSkillRepository.replace_for_userで洗い替え永続化をrun_github_linkの phase C に接続。collect_repos(collect_manifests=True)で直下 manifest を取得。GET /api/github-link/skills+ schemas。OpenAPI 変更に伴いweb/src/api/generated.tsを再生成。レビュー観点(設計判断)
kind判別カラムの単一テーブル+ドメイン層で型分離(言語=byte 比率 / package=宣言+kind)。言語のecosystemは一意制約を効かせるため NULL でなく""(API では null へ正規化)。languages.ymlの完全取り込みは後続。未収録言語は programming として採用(取りこぼし防止、ノイズはexcludeで制御)。テスト / CI
make cigreen(backend lint+test / web lint+test / build)。alembic upgrade headを実 libSQL 経路相当(temp DB)で確認し、0045までクリーンに適用・3 テーブル作成を確認。generated.ts+157 行、skill エンドポイントのみ)。環境変数 / 依存
tomllib)。🤖 Generated with Claude Code
Summary by CodeRabbit