feat(intelligence): monorepo manifest 探索と manifest_path 永続化（ADR-0016 D9）#417
Conversation
recursive Trees API でサブツリーの manifest を探索し、backend/requirements.txt や web/package.json のような分散構成でも依存を拾えるようにする。検出した manifest の相対パスと部分スキャンフラグを github_skill_evidence に永続化する。 - github_collector: recursive Trees API でのパス列挙・除外セグメント （node_modules / vendor 等）・深さ/件数キャップ・partial フラグ伝播 - api_client: fetch_manifest_paths（truncated 判定込み）/ fetch_repo_file - skills: PackageDeclaration.source_path / EvidenceRecord.manifest_path / partial_scan を types・aggregator に伝播 - models / schema: github_skill_evidence に manifest_path・partial_scan を追加 （migration 0046）。OpenAPI 型を再生成 - tests: api_client / collector / aggregator のテストを追加 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 49 minutes and 48 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 ChangesGitHub skill manifest provenance
Sequence Diagram(s)sequenceDiagram
participant CollectManifests as github_collector._collect_manifests
participant FetchManifestPaths as fetch_manifest_paths
participant TreesAPI as GitHub Trees API
participant FetchRepoFile as fetch_repo_file
participant CollectRepos as github_collector.collect_repos
CollectManifests->>FetchManifestPaths: owner, repo, default_branch, manifest_filenames
FetchManifestPaths->>TreesAPI: GET /git/trees/{branch}?recursive=1
TreesAPI-->>FetchManifestPaths: tree entries, truncated
FetchManifestPaths-->>CollectManifests: manifest paths, truncated
CollectManifests->>FetchRepoFile: selected manifest path
FetchRepoFile-->>CollectManifests: manifest contents
CollectManifests-->>CollectRepos: declarations, partial flag
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/adr/0016-github-skill-inference.md (2)
172-173: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConfirm
.terraformexclusion aligns with existingvendor.ymlpatterns.The IaC reuse section proposes adding
.terraformto path exclusions. Consider referencing the existingvendor.ymllink (line 194) more explicitly here, since.terraformis analogous tonode_modules/.venvin D9(b). This would strengthen the "reuse D9 discovery" claim by showing the exclusion pattern is already established.As per path instructions, cross-referencing configuration values across files for logical correctness.
🤖 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/adr/0016-github-skill-inference.md` around lines 172 - 173, In the IaC reuse section, make the `.terraform` path exclusion explicitly align with the existing D9 exclusion pattern by cross-referencing the established `vendor.yml`/`node_modules`/`.venv` style instead of mentioning `.terraform` only in isolation. Update the wording around the D9 reuse claim so the `manifest`/`*.tf` discovery logic and its exclusion set clearly reuse the same path-segmentation approach already described elsewhere in the document, using the relevant D9 and vendor-policy references to anchor the comparison.Source: Path instructions
153-186: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify the
infra_declaredsignal_source value in the context of existing schema constraints.The proposed
infra_declaredvalue forsignal_source(line 168) is consistent with the existing value space (language_bytes,manifest_declared,actual_import) shown inbackend/app/schemas/github_skill.py. However, the ADR should explicitly note that this is a value extension (not requiring a schema migration) sincesignal_sourceis defined as a plainstrin both the ORM model and Pydantic schema, not an enum or constrained set. This is a minor clarity improvement for readers who might assume enum constraints exist.As per the existing schema in
backend/app/schemas/github_skill.py:8-32and model inbackend/app/models/skill.py:87-132,signal_sourceisString(30)/strwithout check constraints.🤖 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/adr/0016-github-skill-inference.md` around lines 153 - 186, Clarify in this ADR section that `infra_declared` is only an added `signal_source` value, not a schema change, by explicitly noting it fits the existing open-ended `signal_source` field used in `Skill` / `github_skill` and does not require a migration. Update the `3 層モデル` / `Layer 2 Evidence` wording so readers understand `infra_declared` is a value-space extension alongside `language_bytes`, `manifest_declared`, and `actual_import`, and point to the existing schema/model design when describing it.backend/app/services/intelligence/github_collector.py (1)
161-161: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale docstring after recursive change.
collect_reposnow scans subtrees recursively, but the docstring still says直下 manifest を解析して(parses root-level manifests only). Update to reflect subtree discovery (D9) to avoid misleading future readers.🤖 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/app/services/intelligence/github_collector.py` at line 161, The docstring for collect_repos is stale after the recursive manifest scan change and still describes only direct child manifests. Update the wording in collect_repos to reflect that manifest discovery now walks subtrees recursively and fills package_declarations from all discovered manifests, so the description matches the current D9 behavior.
🤖 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/services/intelligence/github/api_client.py`:
- Around line 193-208: fetch_manifest_paths() currently swallows non-200 and
httpx.HTTPError cases by returning an empty path list with a false partial flag,
which makes _collect_manifests() treat a failed tree lookup as a clean
no-manifest result. Update fetch_manifest_paths() to thread an explicit
failure/partial signal through those error paths, and make sure
_collect_manifests() uses that signal so manifest_scan_partial is set true
whenever the tree fetch does not complete successfully.
---
Nitpick comments:
In `@backend/app/services/intelligence/github_collector.py`:
- Line 161: The docstring for collect_repos is stale after the recursive
manifest scan change and still describes only direct child manifests. Update the
wording in collect_repos to reflect that manifest discovery now walks subtrees
recursively and fills package_declarations from all discovered manifests, so the
description matches the current D9 behavior.
In `@docs/adr/0016-github-skill-inference.md`:
- Around line 172-173: In the IaC reuse section, make the `.terraform` path
exclusion explicitly align with the existing D9 exclusion pattern by
cross-referencing the established `vendor.yml`/`node_modules`/`.venv` style
instead of mentioning `.terraform` only in isolation. Update the wording around
the D9 reuse claim so the `manifest`/`*.tf` discovery logic and its exclusion
set clearly reuse the same path-segmentation approach already described
elsewhere in the document, using the relevant D9 and vendor-policy references to
anchor the comparison.
- Around line 153-186: Clarify in this ADR section that `infra_declared` is only
an added `signal_source` value, not a schema change, by explicitly noting it
fits the existing open-ended `signal_source` field used in `Skill` /
`github_skill` and does not require a migration. Update the `3 層モデル` / `Layer 2
Evidence` wording so readers understand `infra_declared` is a value-space
extension alongside `language_bytes`, `manifest_declared`, and `actual_import`,
and point to the existing schema/model design when describing it.
🪄 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: e0cd4150-2129-4390-96cb-25f5f3353ebe
📒 Files selected for processing (16)
backend/alembic_migrations/versions/0046_add_manifest_path_to_github_skill_evidence.pybackend/app/models/skill.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/aggregator.pybackend/app/services/intelligence/skills/types.pybackend/tests/test_github_api_client.pybackend/tests/test_github_collector_extended.pybackend/tests/test_github_skills_api.pybackend/tests/test_skill_aggregator.pydocs/adr/0016-github-skill-inference.mdweb/src/api/generated.ts
fetch_manifest_paths が非200 / 不正レスポンス / httpx.HTTPError 時に ([], False) を返していたため、_collect_manifests が「取得失敗」と 「manifest が本当に無い」を区別できず manifest_scan_partial を立てられて いなかった。失敗時は partial=True を返し、依存ゼロの過信を防ぐ（D9(d)）。 不正 owner/repo は走査対象ですらないため ([], False) を維持。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
概要
ADR-0016 の D9（monorepo サブツリー manifest 探索） を実装し、
backend/requirements.txtやweb/package.jsonのようにサブツリーへ分散する構成でも依存を拾えるようにする。あわせて、検出した manifest の相対パスと部分スキャンフラグをgithub_skill_evidenceに永続化し、証跡性を強化する。変更内容
node_modules/vendor/.venv等）、深さ（D9c）・件数キャップ、partial フラグ伝播fetch_manifest_paths（truncated 判定込み）/fetch_repo_filePackageDeclaration.source_path/EvidenceRecord.manifest_path/partial_scanを types・aggregator へ伝播github_skill_evidenceにmanifest_path/partial_scanを追加（migration0046）。OpenAPI 型を再生成（web/src/api/generated.ts）確認事項
make cigreen（backend lint/test・web lint/test 388 pass・web build）make codegen-types再生成済み・ドリフトなし0046は ADD COLUMN（libSQL 対応）。pytest はマイグレーションを通らないため、実適用は CIsmoke-backendで検証される🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes