feat(intelligence): GitHub スキル推論に verify ステージ（import 解析）を追加 - #445
Merged
Conversation
ADR-0016 D6 を実装。direct 依存が実際に import されているかをソース解析で確認し、 manifest_declared を actual_import 証跡へ昇格させる（昇格のみ・降格なし）。 - api_client: fetch_repo_tree を新設し recursive Trees を 1 コール化。 manifest 探索と import 解析が同一ツリーを共有（tree 再取得しない / D9(a)） - skills/imports/: ImportScanner plugin（go/python/js_ts/rust）。辞書レスの 保守的照合（-→_ 変換 / go 接頭辞一致 / npm 完全一致）。取りこぼしは false negative 受容 - github_collector: _collect_repo_signals で manifest 宣言 + direct エコシステムの source を除外/浅い順/件数キャップ(30)でサンプリング→import 解析。生コードは scan 後破棄 - aggregator: direct かつ import 検出時に actual_import 証跡(confidence 0.85)を追加 - テスト追加（import スキャナ / aggregator verify / collector / api_client tree） API スキーマ・型は不変（signal_source 値域内）。migration / codegen drift なし。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Warning Review limit reached
Next review available in:42 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughImplements the D6 verify stage for GitHub skill inference: introduces an ChangesD6 Verify Stage: Import Scanner Plugins and actual_import Evidence
Sequence Diagram(s)sequenceDiagram
participant collect_repos
participant _collect_repo_signals
participant fetch_repo_tree as GitHub Trees API
participant parse_manifest
participant ImportScanner
collect_repos->>_collect_repo_signals: repo metadata (owner/repo/branch)
_collect_repo_signals->>fetch_repo_tree: recursive tree request
fetch_repo_tree-->>_collect_repo_signals: (all_blob_paths, truncated)
_collect_repo_signals->>parse_manifest: manifest file contents
parse_manifest-->>_collect_repo_signals: package_declarations (with dependency_kind)
_collect_repo_signals->>ImportScanner: source files for direct-dep ecosystems only
ImportScanner-->>_collect_repo_signals: imported_symbols per ecosystem
_collect_repo_signals-->>collect_repos: (decls, imported_symbols, scan_partial)
Note over collect_repos: RepoData.imported_symbols populated
collect_repos->>aggregator: RepoSkillInput(imported_symbols=...)
aggregator->>ImportScanner: scanner_for_ecosystem + matches(canonical_name, imported)
ImportScanner-->>aggregator: bool
aggregator-->>collect_repos: EvidenceRecord(actual_import) appended if matched
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/app/services/intelligence/github_collector.py (1)
149-168: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptional: avoid the duplicate
scanner_for_extensionlookup.
scanner_for_extension(path)is computed once to buildsource_candidatesand recomputed inside the selection loop. You can carry the scanner alongside the path to avoid the second lookup.♻️ Possible refactor
- source_candidates = []+ source_candidates: list[tuple[str, object]] = [] for path in tree_paths: scanner = scanner_for_extension(path) if scanner is not None and scanner.ecosystem in direct_ecosystems: - source_candidates.append(path)- selected_sources, source_dropped = _select_shallow(- source_candidates, _SOURCE_MAX_DEPTH, _SOURCE_MAX_COUNT- )- for path in selected_sources:- scanner = scanner_for_extension(path)- if scanner is None:- continue+ source_candidates.append((path, scanner))+ selected_paths, source_dropped = _select_shallow(+ [p for p, _ in source_candidates], _SOURCE_MAX_DEPTH, _SOURCE_MAX_COUNT+ )+ scanner_by_path = dict(source_candidates)+ for path in selected_paths:+ scanner = scanner_by_path[path] content = await fetch_repo_file(client, owner, repo, path) if not content: continue imported_symbols.setdefault(scanner.ecosystem, set()).update( scanner.scan(content) )🤖 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` around lines 149 - 168, Avoid recomputing scanner_for_extension(path) in the direct_ecosystems path selection flow: in the logic that builds source_candidates and then iterates selected_sources, carry the scanner together with each path so the same scanner object can be reused after _select_shallow. Update the selected_sources loop in github_collector.py to use the precomputed scanner instead of looking it up again, while keeping the existing fetch_repo_file and imported_symbols update behavior unchanged.
🤖 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/skills/imports/js_ts.py`:
- Around line 12-14: The import scanner in js_ts.py is matching require/import
text inside comments, docs, and string literals, which can create false
positives in imported_symbols and incorrectly mark packages as actual imports.
Update the extraction logic around _SPEC_RE and the JS/TS import parsing path to
ignore comments and string contents before matching, preferably by using a
token/AST-based approach or a preprocessing step that strips non-code regions
before applying the regex.
In `@backend/app/services/intelligence/skills/imports/rust.py`:
- Around line 11-24: Update the Rust import scanner so `RustImportScanner.scan`
also detects re-exported imports like `pub use` and `pub(crate) use`, not just
plain `use` statements. Adjust `_USE_RE` in the Rust import module to allow an
optional visibility prefix before `use`, while keeping the existing
`RustImportScanner`, `_USE_RE`, and `_NON_CRATE` behavior unchanged for
filtering and external-crate detection.
---
Nitpick comments:
In `@backend/app/services/intelligence/github_collector.py`:
- Around line 149-168: Avoid recomputing scanner_for_extension(path) in the
direct_ecosystems path selection flow: in the logic that builds
source_candidates and then iterates selected_sources, carry the scanner together
with each path so the same scanner object can be reused after _select_shallow.
Update the selected_sources loop in github_collector.py to use the precomputed
scanner instead of looking it up again, while keeping the existing
fetch_repo_file and imported_symbols update behavior unchanged.
🪄 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: 5fe0422c-62e3-4ccd-9bcc-be3c58ff04ba
📒 Files selected for processing (17)
.claude/rules/backend/architecture.mdbackend/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/imports/__init__.pybackend/app/services/intelligence/skills/imports/base.pybackend/app/services/intelligence/skills/imports/go.pybackend/app/services/intelligence/skills/imports/js_ts.pybackend/app/services/intelligence/skills/imports/python.pybackend/app/services/intelligence/skills/imports/registry.pybackend/app/services/intelligence/skills/imports/rust.pybackend/tests/test_github_api_client.pybackend/tests/test_github_collector_extended.pybackend/tests/test_skill_aggregator.pybackend/tests/test_skill_import_scanners.pydocs/adr/0016-github-skill-inference.md
Comment on lines
+12
to
+14
| _SPEC_RE = re.compile( | ||
| r"""(?:\bfrom|\bimport|\brequire)\s*\(?\s*["']([^"']+)["']""", | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'\n== file outline ==\n'
ast-grep outline backend/app/services/intelligence/skills/imports/js_ts.py --view expanded ||trueprintf'\n== target file excerpt ==\n'
nl -ba backend/app/services/intelligence/skills/imports/js_ts.py | sed -n '1,220p'printf'\n== search downstream usages ==\n'
rg -n "imported_symbols|manifest_declared|actual_import|_SPEC_RE" backend/app/services -S
printf'\n== related tests ==\n'
rg -n "exclude.*relative|imported_symbols|manifest_declared|actual_import|require\\(" backend -g '*test*' -g '*spec*' -SRepository: yusuke0610/devforge
Length of output: 632
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'\n== target file ==\n'
sed -n '1,220p' backend/app/services/intelligence/skills/imports/js_ts.py | cat -n
printf'\n== related code ==\n'
rg -n "imported_symbols|manifest_declared|actual_import|JsTsImportScanner|_SPEC_RE" backend/app/services -S
printf'\n== tests ==\n'
rg -n "JsTsImportScanner|imported_symbols|manifest_declared|actual_import|require\\(" backend -g '*test*' -g '*spec*' -SRepository: yusuke0610/devforge
Length of output: 6555
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'\n== scanner tests ==\n'
sed -n '1,220p' backend/tests/test_skill_import_scanners.py | cat -n
printf'\n== aggregator tests around actual_import ==\n'
sed -n '160,260p' backend/tests/test_skill_aggregator.py | cat -n
printf'\n== regex probe ==\n'
python3 - <<'PY'import respec_re = re.compile(r'''(?:\bfrom|\bimport|\brequire)\s*\(?\s*["']([^"']+)["']''')cases = { "line_comment": "// require(\"react\")", "block_comment": "/* import 'left-pad' */", "string_literal": "const s = 'import(\"lodash\")'", "real_require": "const x = require('lodash');", "real_import": "import {x} from '`@scope/pkg/subpath`';", "dynamic_import": "const m = import('vite');", "example_text": "docs: import('react') in README",}for name, text in cases.items(): print(name, spec_re.findall(text))PYRepository: yusuke0610/devforge
Length of output: 8860
Strip comments and string literals before scanning JS/TS importsbackend/app/services/intelligence/skills/imports/js_ts.py:12-40
This regex also matches text inside comments, docs, and string literals (// require("react"), 'import("lodash")'), so imported_symbols can include packages that never appear in real import syntax and wrongly promote manifest_declared to actual_import. Switch to token/AST-based extraction, or preprocess the file to exclude comments and strings before matching.
🤖 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/skills/imports/js_ts.py` around lines 12 -
14, The import scanner in js_ts.py is matching require/import text inside
comments, docs, and string literals, which can create false positives in
imported_symbols and incorrectly mark packages as actual imports. Update the
extraction logic around _SPEC_RE and the JS/TS import parsing path to ignore
comments and string contents before matching, preferably by using a
token/AST-based approach or a preprocessing step that strips non-code regions
before applying the regex.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'\n== git head ==\n'
git rev-parse --short HEAD
printf'\n== file outline ==\n'
ast-grep outline backend/app/services/intelligence/skills/imports/js_ts.py --view expanded ||trueprintf'\n== current js_ts.py ==\n'
nl -ba backend/app/services/intelligence/skills/imports/js_ts.py | sed -n '1,220p'printf'\n== related scanner tests ==\n'
nl -ba backend/tests/test_skill_import_scanners.py | sed -n '1,260p'printf'\n== search for new comment/string stripping logic ==\n'
rg -n -C2 "comment|string|strip|token|lexer|scan\(" backend/app/services/intelligence/skills/imports backend/tests/test_skill_import_scanners.py -SLength of output: 1624
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'\n== current js_ts.py ==\n'
cat -n backend/app/services/intelligence/skills/imports/js_ts.py | sed -n '1,220p'printf'\n== JS/TS scanner tests excerpt ==\n'
cat -n backend/tests/test_skill_import_scanners.py | sed -n '40,180p'printf'\n== focused grep around false-positive coverage ==\n'
rg -n -C3 "comment|string literal|string_literal|relative_imports_excluded|require\\(|import\\(" backend/tests/test_skill_import_scanners.py backend/app/services/intelligence/skills/imports/js_ts.py -SLength of output: 14549
@yusuke0610, 再RVしました。
- コメント内の誤検出は解消されています。
backend/app/services/intelligence/skills/imports/js_ts.pyの Line 16-Line 28 でコメント除去が入り、Line 50 でそれを通してから走査しています。 - そのうえで、
backend/tests/test_skill_import_scanners.pyの Line 90-Line 97 に回帰テストも追加されていました。
一方で、string literal 内の import("x") / require("x") 風テキストについては、backend/app/services/intelligence/skills/imports/js_ts.py の Line 25-Line 26 にある通り、現状は意図的に未対応です。
なので結論としては：
- 元コメントのうち 「comments/docs で誤検出する」部分は解消済み
- 「string literals でも誤検出しうる」部分はまだ残っている
という状態です。
必要なら、元の指摘は少し広すぎたので、コメント起因の部分は取り下げて、string literal 限定の懸念に絞るのが正確です。
🧠 Learnings used
Learnt from: yusuke0610
Repo: yusuke0610/devforge PR: 265
File: backend/app/routers/resume_imports.py:165-168
Timestamp: 2026-05-22T11:50:08.476Z
Learning: Follow the devforge project rule in `.claude/CLAUDE.md`: during code review, do NOT ask for extra error handling/fallbacks/defensive validation for states that are guaranteed not to occur by system invariants. Treat data written by the system’s own handlers within the same DB transaction (and tied to the same commit/invariant—e.g., fields associated with `status="completed"`) as trusted. Apply validation only at system boundaries (user input and external APIs) or when data can change across transaction boundaries / originate outside the trusted handler path. Example: keep `json.loads(record.result_json)` unguarded in `backend/app/routers/resume_imports.py` when `result_json` is guaranteed valid by the same handler that sets the corresponding completion status.
Uh oh!
There was an error while loading. Please reload this page.
- js_ts: コメント（// と /* */）を除去してから import を走査。コメント内の import/require 誤検出による actual_import 誤昇格を防止（URL の // は保護） - rust: pub use / pub(crate) use の re-export も外部クレート使用として拾うよう visibility 接頭辞を許容 - github_collector: verify の source 選定で scanner_for_extension の二重計算を解消 （path→scanner を一度引いて持ち回る） - スキャナ改善に対応するテストを追加 挙動は精度向上のみ。API 契約・スキーマは不変。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jun 30, 2026
This was referenced Jul 8, 2026
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
概要
ADR-0016「GitHub 連携によるスキル推論基盤」の残タスク タスク1（verify ステージ / D6） を実装。direct 依存が実際に import されているかをソース解析で確認し、
manifest_declared（宣言）をactual_import（実使用）証跡へ昇格させる（昇格のみ・降格なし。declare 証跡は保持 / D8「保持は細かく」）。declare までで経歴書は出力可能、verify は精度を後追いで上げる位置づけ（D5）。
変更内容
github/api_client.py:fetch_manifest_pathsをfetch_repo_tree（recursive Trees を 1 コールで全 blob パス取得）に分離。manifest 探索（declare）と import 解析（verify）が同一ツリーを共有し、verify のために tree を再取得しない（D9(a) 維持）skills/imports/:ImportScannerplugin（go/python/js_ts/rust）+ registry。辞書を持たず（D3）、エコシステム別の保守的照合で実 import を判定:module/...接頭辞一致@scope/name/ subpath 対応）-→_変換後の一致PyYAML→yaml等）は昇格漏れ（false negative）として受容（テストで明示固定）github_collector.py:_collect_manifestsを_collect_repo_signalsに拡張。1 ツリーから manifest 宣言と、direct 宣言のあるエコシステムの source を除外セグメント/浅い順/件数キャップ（30件）でサンプリング → import 解析。生コードは scan 後に破棄（永続化しない / D6）skills/aggregator.py:RepoSkillInput.imported_symbolsを追加。direct かつ import 検出時にactual_import証跡（confidence 0.85）を追加test_skill_import_scanners.py）/ aggregator verify / collector verify / api_client tree判断事項
GET /api/github-link/skillsの evidence に direct 依存でactual_importが増える（既存manifest_declaredは残存）signal_sourceは既存値域内（actual_importは model/schema に既記載）→ migration 不要・codegen drift なしテスト
make cigreen（backend lint+test / web / build）補足（別 PR 予定）
skills/imports/およびskills/manifests/の Protocol 属性に対する pyright 型注釈（extensions: tuple[str, ...]等の invariance 対応）は、本 PR と分離して別 PR で対応します。🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes