triage: deterministic dependency-manifest resolver (dep_manifest + Lane A)#322
Merged
Conversation
|
Warning Review limit reached
Next review available in: 11 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: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughAdds a deterministic ChangesDependency manifest artifact and triage integration
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Scanner as Scanner Finding
participant TriageRunner as triage_runner
participant DepResolver as resolve_by_dep_manifest
participant DepManifest as dep_manifest artifact
participant DeepDive as maybe_deep_dive
Scanner->>TriageRunner: dependency finding (package, version)
TriageRunner->>DepManifest: _load_dep_manifest(repo)
DepManifest-->>TriageRunner: dep_manifest dict
TriageRunner->>DepResolver: resolve_by_dep_manifest(finding, dep_manifest)
alt scope/import checks pass
DepResolver-->>TriageRunner: TriageOutput(false_positive)
TriageRunner-->>Scanner: cleared verdict (no deep dive)
else checks fail or ambiguous
DepResolver-->>TriageRunner: None
TriageRunner->>DeepDive: maybe_deep_dive(repo_knowledge incl. dep_manifest)
DeepDive-->>TriageRunner: deep dive verdict
end
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
…sks 1,3) dep_manifest artifact foundation: DepNode/DepManifest pydantic schemas (lockfile-resolved graph keyed by (name,version)) and DepGraph with prod-dominates scope propagation. Validated against the karakeep undici@6.23.0-prod vs undici@7.16.0-transitive version split. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… 2,6) manifests.classify_scope (conservative: dir dominates, unknown->prod) and imports.find_import_sites (version-agnostic repo-wide import grep, excludes test/docs, substring-safe). These are the two independent safety gates: a dep-only clear requires BOTH non-prod scope AND zero shipping import sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Clears a dependency finding as false_positive iff its (name,version) node reaches only dev/test/docs/build roots (no prod/optional) AND has zero first-party shipping import sites AND a resolved import name. Two independent safety gates + conservative fallthrough on any ambiguity (collision, unknown scope, version mismatch). Structural clear like code_map (tier-independent). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bucket all first-party imports by package/import name in a single tree walk, so the assembler does O(1) lookups per node instead of re-walking per node. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…P1 tasks 4,5,7) npm.py: pnpm-lock v9, yarn.lock (classic+berry), package-lock v2/v3; version- split verified (karakeep undici 6.23.0 prod vs 7.16.0 — 7.16.0 genuinely ships via metascraper->cheerio). pypi.py: uv.lock/poetry.lock/Pipfile.lock + declared- only fallback + dist->import map. build.py assembles per (name,version) DepNodes with propagated scopes + one-pass import sites. Parsers over-connect on ambiguity (safety). 69 deps tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ask 9) ARTIFACTS += dep_manifest (deterministic builder in make_profile_builders); RepoKnowledge.dep_manifest; triage_runner dep_manifest gate (type==dependency, pkg@ver from raw_payload) between the code_map gate and the Deep dive; thread dep_manifest into the Deep dive knowledge + stage prior_context. +2 gate tests (dev-only clears/skips deep dive; prod dep falls through). Existing 3-artifact assertions updated. 202 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A [project.optional-dependencies] extra named dev/docs/test/lint/... is non-shipping tooling, not a shippable feature extra (anthropic, postgres). classify_extra() maps by name; the uv.lock parser applies it. Recovers instructor's dev/docs tools (pytest/black/mkdocs/cairosvg + transitive nbconvert/mistune): Lane A gold clears 104 -> 128, still 0 non-noise. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Real shipping code lives in scripts/ dirs (e.g. karakeep's worker entrypoint apps/workers/scripts/parseHtmlSubprocess.ts imports dompurify). Excluding them false-negatived 'not imported' — a latent Lane A safety gap and a Lane B recall loss. Lane A gold gate unchanged (128 cleared, 0 non-noise). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A dep imported only in vite.config.ts / webpack.config.js / .eslintrc.js runs at build time, not in the shipped app, so it must not count as a shipping import (would block a safe dev-only clear). Lane A gold gate unchanged (128, 0 non-noise) — the affected finding versions ship transitively anyway — but it removes a latent over-block and sharpens Lane B's imported_firstparty signal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
galanko
force-pushed
the
feat/triage-dep-manifest
branch
from
July 5, 2026 09:37
01c6879 to
3c51f1e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 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/cliff/agents/triage_dep_manifest.py`:
- Around line 104-105: The unresolved import-name guard in
triage_dep_manifest.py has an overly long inline comment that triggers ruff
E501; split the explanatory comment off the return statement in the logic around
the import_name check so the line stays within the limit. Keep the behavior in
the same branch of the code path that returns None when node.get("import_name")
is missing, but move the pitfall explanation to a separate comment line.
- Around line 93-98: The shipping-scope safety gate in triage_dep_manifest
currently uses _SHIPPING_SCOPES as a denylist, so unknown scope values can
incorrectly be treated as safe. Update the scope handling in the resolver logic
around the scopes check to use an allowlist of known non-shipping scopes
instead, and only return None when every scope is explicitly recognized as
non-shipping. Keep the guard behavior in the same decision path that reads
node["scopes"], so untyped JSON with unexpected or stale scope values is treated
as unsafe and not cleared.
In `@backend/cliff/agents/triage_runner.py`:
- Around line 272-287: The dep_manifest gate in triage_runner duplicates the
same load → resolve → persist_and_return flow used by the code_map gate, so
consider refactoring that shared pattern into a small helper to reduce
repetition. Extract the common orchestration around `_load_dep_manifest`,
`resolve_by_dep_manifest`, `_persist_synthesis`, and the analogous code_map path
into a reusable function (for example, a `_try_gate`-style helper), while
keeping the distinct preconditions in the existing gate branches.
In `@backend/cliff/repos/deps/graph.py`:
- Around line 20-46: Update the DepGraph root modeling to use the existing
DepScope type instead of plain str for scope. In the Root dataclass and
DepGraph.add_root, replace the scope annotation so graph construction is
constrained to the valid literal values already defined in schemas.DepScope, and
keep the rest of the add_root flow unchanged.
- Line 40: The add_root method signature in graph.py exceeds the Ruff E501
line-length limit. Reformat the def add_root(...) declaration by breaking the
parameters across multiple lines so it stays within 100 characters, while
keeping the method name and parameter order unchanged.
- Line 15: Remove the unused dataclasses import in graph.py by deleting field
from the import list; only keep the symbols actually referenced in this module.
Verify the top-level imports in the graph module and run ruff so the F401 lint
error is cleared.
In `@backend/cliff/repos/deps/imports.py`:
- Line 64: The Python import parser in _PY_IMPORT_RE only captures the first
imported name on a comma-separated import line, so collect_import_sites misses
later symbols. Update the import handling in imports.py to recognize and record
every name from statements like import a, b, c, using the existing parsing flow
around _PY_IMPORT_RE and collect_import_sites so all imported modules are
preserved.
In `@backend/cliff/repos/deps/manifests.py`:
- Line 104: The _SKIP_DIRS set declaration in manifests.py exceeds the ruff E501
line limit and should be wrapped to fit within 100 characters. Reformat the
_SKIP_DIRS assignment by splitting the long set across multiple lines while
keeping the same directory names and ordering style consistent with the rest of
the module.
- Around line 17-18: The manifest constants in the deps manifest module exceed
Ruff’s E501 limit and need to be shortened. Reformat the _DOCS_DIRS and
_TEST_DIRS assignments so each line stays within 100 characters, keeping the
same set values while preserving readability; use the existing constant names to
locate the edit.
- Around line 110-131: discover_manifests currently filters subdirectories with
a case-sensitive _SKIP_DIRS check, which is inconsistent with the import
scanner. Update the os.walk dirnames filtering in discover_manifests to compare
directory names case-insensitively, matching imports.py’s _EXCLUDE_DIRS logic,
so differently cased excluded folders are skipped consistently. Use the
discover_manifests function and the _SKIP_DIRS constant as the fix location.
In `@backend/cliff/repos/deps/npm.py`:
- Line 112: Fix the ruff E501 violations by wrapping the long function
signatures and expressions in backend/cliff/repos/deps/npm.py, including
parse_pnpm_lock, _parse_yarn_berry, _parse_yarn_classic, the _resolve_range
calls, _add, and the pkg_key ternary. Keep the same logic but split parameters
and long expressions across multiple lines so each affected definition/call
stays within the 100-character limit.
- Around line 399-403: _read_package_json currently hides unreadable or invalid
package.json files by returning an empty dict, which makes callers treat them
like dependency-free manifests. Update _read_package_json in npm.py to signal
failure distinctly (for example by returning None or raising a handled
parse/read error), then adjust the package.json, yarn.lock, and
package-lock.json manifest loops to add those entries to unresolved and skip
_declared_deps() when the manifest could not be read or parsed. Ensure
_declared_deps, _add_dep, and the governed-manifest handling paths preserve real
dependencies instead of silently dropping them.
- Around line 440-447: The ancestor check in `_nearest_lock_dir` is hard to read
because the `or`/ternary precedence is implicit, so make the root-case and
path-prefix logic explicit. Refactor the condition inside `_nearest_lock_dir`
into a clearly parenthesized form or equivalent guard clauses so it unmistakably
means “empty lock dir is always valid, otherwise it must match exactly or be a
path ancestor,” while keeping the existing `best` selection logic unchanged.
- Around line 39-42: Merge the duplicate imports from the .manifests module in
the npm dependency loader so the import block passes ruff I001; keep the
existing symbols used by the module, but combine the Manifest, classify_scope,
discover_manifests, and _SKIP_DIRS imports into a single from .manifests import
statement near DepGraph, without changing behavior.
In `@backend/cliff/repos/deps/pypi.py`:
- Line 207: Fix the Ruff E501 failures by wrapping the long expressions and
signatures in pypi.py so they stay within the 100-character limit. Update the
affected helpers and loops such as _uv_roots, _poetry_roots_from_pyproject, the
unresolved.append error strings, and the tuple-unpacking for loops by splitting
arguments/conditions across multiple lines while preserving behavior.
- Around line 530-543: The `resolved` flag in `parse_pypi_deps` is currently
shared across the entire repo, so a successful parse in one lock directory skips
`_parse_poetry_lock`, `_parse_pipfile_lock`, and `_parse_declared` for other
sub-projects. Update the logic around the `uv_locks`, `poetry_locks`,
`pipfile_locks`, and `_parse_declared` branches to track resolution per lock
directory (or per project group) instead of one global boolean, so each
directory still gets the appropriate parser/fallback. Keep the existing helper
calls (`_parse_uv_lock`, `_parse_poetry_lock`, `_parse_pipfile_lock`,
`_parse_declared`) but ensure one directory’s success does not suppress
dependency parsing for other directories.
In `@backend/cliff/repos/profile_agents.py`:
- Around line 109-117: The _build coroutine in make_dep_manifest is async but
does all work synchronously by calling build_dep_manifest(clone_dir) directly,
which can block the event loop. Update the ProfileBuilder returned from
make_dep_manifest so the manifest build runs off the event loop, preferably by
delegating the build_dep_manifest call to a worker thread or equivalent
executor, while keeping the same ProfileRunner-facing async signature.
In `@backend/cliff/repos/schemas.py`:
- Line 83: `DepManifest` is using overly broad string types for ecosystem
values; tighten the schema to the same strict typing style used by `DepScope`.
Update the `ecosystem` field and the `DepManifest.ecosystems` collection in
`schemas.py` to use `Literal["npm", "pypi"]` (and a matching list/collection of
that literal type) so only the documented values are accepted and typos are
caught at model construction time.
In `@backend/tests/agents/test_triage_dep_manifest.py`:
- Around line 12-22: The _node helper signature exceeds the line-length limit
and needs to be wrapped to satisfy ruff E501. Reformat the _node function
definition in test_triage_dep_manifest.py by splitting the parameters across
multiple lines while keeping the same defaults and keyword-only arguments, and
preserve the existing return structure unchanged.
In `@backend/tests/repos/deps/test_imports.py`:
- Around line 71-82: Extend test_build_config_import_not_counted in
test_imports.py to cover the two missed cases from imports.py: add a generic
app.config.js runtime file with a real source import so it is counted as a
shipping import, and add a Python import fixture using comma-separated names
such as import os, sys, requests to verify _PY_IMPORT_RE and the import-site
collectors handle multiple imports on one line. Use find_import_sites and
collect_import_sites to assert the new cases behave correctly alongside the
existing build-config exclusions.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f01347da-0a3b-4d7b-b6f4-0d75ee1efe14
⛔ Files ignored due to path filters (5)
backend/tests/repos/deps/fixtures/npm/package-lock/package-lock.jsonis excluded by!**/package-lock.jsonbackend/tests/repos/deps/fixtures/npm/pnpm/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlbackend/tests/repos/deps/fixtures/npm/yarn-berry/yarn.lockis excluded by!**/yarn.lock,!**/*.lockbackend/tests/repos/deps/fixtures/npm/yarn-classic/yarn.lockis excluded by!**/yarn.lock,!**/*.lockbackend/tests/repos/deps/fixtures/pypi/uvslice/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
backend/cliff/agents/triage_deep/integration.pybackend/cliff/agents/triage_deep/runner.pybackend/cliff/agents/triage_dep_manifest.pybackend/cliff/agents/triage_runner.pybackend/cliff/repos/deps/__init__.pybackend/cliff/repos/deps/build.pybackend/cliff/repos/deps/graph.pybackend/cliff/repos/deps/imports.pybackend/cliff/repos/deps/manifests.pybackend/cliff/repos/deps/npm.pybackend/cliff/repos/deps/pypi.pybackend/cliff/repos/knowledge.pybackend/cliff/repos/profile_agents.pybackend/cliff/repos/repo_dir_manager.pybackend/cliff/repos/schemas.pybackend/tests/agents/test_deep_dive_integration.pybackend/tests/agents/test_triage_dep_manifest.pybackend/tests/agents/test_triage_runner.pybackend/tests/repos/deps/__init__.pybackend/tests/repos/deps/fixtures/npm/package-lock/package.jsonbackend/tests/repos/deps/fixtures/npm/yarn-berry/apps/web/package.jsonbackend/tests/repos/deps/fixtures/npm/yarn-classic/package.jsonbackend/tests/repos/deps/fixtures/pypi/reqdoc/requirements-doc.txtbackend/tests/repos/deps/test_graph.pybackend/tests/repos/deps/test_imports.pybackend/tests/repos/deps/test_manifests.pybackend/tests/repos/deps/test_npm.pybackend/tests/repos/deps/test_pypi.pybackend/tests/repos/test_dep_schemas.pybackend/tests/repos/test_profile_agents.pybackend/tests/repos/test_service.py
Comment on lines
+272
to
+287
| # Deterministic dep_manifest gate (SP1): clear a dependency finding whose | ||
| # package provably does not ship (dev/test-only, unimported). The dep finding | ||
| # carries pkg@version in raw_payload; code_map's file-path gate never matches | ||
| # it, so this runs only for type=="dependency" and is a pure structural clear. | ||
| if finding.type == "dependency": | ||
| rp = finding.raw_payload or {} | ||
| pkg, ver = rp.get("package"), rp.get("version") | ||
| if pkg and ver: | ||
| dep_manifest = await _load_dep_manifest(db, workspace.repo_url) | ||
| dep_cleared = resolve_by_dep_manifest( | ||
| {**finding_ctx, "location": f"{pkg}@{ver}"}, dep_manifest | ||
| ) | ||
| if dep_cleared is not None: | ||
| await _persist_synthesis(db, workspace.id, dep_cleared) | ||
| return dep_cleared | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Deterministic dep_manifest gate correctly gated and wired.
Confirmed against the gate test that monkeypatches _load_dep_manifest to a dev-only, unimported node and asserts the run_triage verdict is false_positive while maybe_deep_dive is not called, and the symmetric prod-dependency fall-through test. Logic is sound.
Minor observation (optional): this block repeats the same "load → resolve → persist+return" shape as the code_map gate above it (Lines 266-270). Not urgent given each has a distinct precondition, but if a third gate is added later, consider extracting a small _try_gate(resolver, ctx, loader) helper.
🤖 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/cliff/agents/triage_runner.py` around lines 272 - 287, The
dep_manifest gate in triage_runner duplicates the same load → resolve →
persist_and_return flow used by the code_map gate, so consider refactoring that
shared pattern into a small helper to reduce repetition. Extract the common
orchestration around `_load_dep_manifest`, `resolve_by_dep_manifest`,
`_persist_synthesis`, and the analogous code_map path into a reusable function
(for example, a `_try_gate`-style helper), while keeping the distinct
preconditions in the existing gate branches.
Comment on lines
+20
to
+46
| @dataclass(frozen=True) | ||
| class Root: | ||
| name: str | ||
| version: str | ||
| scope: str # prod | dev | test | docs | build | optional | ||
| declared_in: str # manifest path | ||
|
|
||
|
|
||
| class DepGraph: | ||
| def __init__(self) -> None: | ||
| self._nodes: dict[NV, str] = {} # (name, version) -> ecosystem | ||
| self._edges: dict[NV, set[NV]] = {} # (name, version) -> deps | ||
| self.roots: list[Root] = [] | ||
|
|
||
| # ── construction ──────────────────────────────────────────────────────── | ||
| def add_node(self, name: str, version: str, ecosystem: str) -> None: | ||
| nv = (name, version) | ||
| self._nodes.setdefault(nv, ecosystem) | ||
| self._edges.setdefault(nv, set()) | ||
|
|
||
| def add_root(self, name: str, version: str, scope: str, declared_in: str, ecosystem: str) -> None: | ||
| self.add_node(name, version, ecosystem) | ||
| self.roots.append(Root(name=name, version=version, scope=scope, declared_in=declared_in)) | ||
|
|
||
| def add_edge(self, frm: NV, to: NV) -> None: | ||
| self._edges.setdefault(frm, set()).add(to) | ||
| self._edges.setdefault(to, set()) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Consider typing scope as DepScope instead of str.
Root.scope and add_root's scope parameter are plain str, but schemas.DepScope already defines the exact literal set of valid values. Reusing it here would catch invalid scope strings (typos) at graph-construction time rather than only when the value later flows into a DepNode.
As per coding guidelines, "Python style: use strict type hints and Pydantic models (ruff enforced)".
🧰 Tools
🪛 GitHub Actions: Backend CI / 0_lint-and-test.txt
[error] 40-40: Ruff E501: Line too long (102 > 100).
🪛 GitHub Actions: Backend CI / lint-and-test
[error] 40-40: ruff (E501) Line too long (102 > 100). Line: def add_root(... ) -> None:
🤖 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/cliff/repos/deps/graph.py` around lines 20 - 46, Update the DepGraph
root modeling to use the existing DepScope type instead of plain str for scope.
In the Root dataclass and DepGraph.add_root, replace the scope annotation so
graph construction is constrained to the valid literal values already defined in
schemas.DepScope, and keep the rest of the add_root flow unchanged.
Source: Coding guidelines
Comment on lines
+71
to
+82
| def test_build_config_import_not_counted(tmp_path) -> None: | ||
| # a dep imported only in a build-config file does not ship → not a shipping import | ||
| (tmp_path / "vite.config.ts").write_text("import react from '@vitejs/plugin-react'\n") | ||
| (tmp_path / "webpack.config.js").write_text("const x = require('terser-webpack-plugin')\n") | ||
| (tmp_path / ".eslintrc.js").write_text("module.exports = require('eslint-config-foo')\n") | ||
| assert find_import_sites(tmp_path, "@vitejs/plugin-react", "npm") == [] | ||
| assert find_import_sites(tmp_path, "terser-webpack-plugin", "npm") == [] | ||
| assert collect_import_sites(tmp_path, "npm") == {} | ||
| # but a real source import IS counted | ||
| (tmp_path / "app.ts").write_text("import react from '@vitejs/plugin-react'\n") | ||
| assert find_import_sites(tmp_path, "@vitejs/plugin-react", "npm") == ["app.ts:1"] | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add coverage for generic *.config.js runtime files and comma-separated Python imports.
Current tests only exercise named build tools (vite.config.ts, webpack.config.js, .eslintrc.js) and single-name Python imports. Given the correctness gaps flagged in imports.py (broad _BUILD_CONFIG_RE glob and _PY_IMPORT_RE missing multi-name imports), adding cases like a non-build app.config.js with a real runtime import, and import os, sys, requests, would catch regressions in this safety-critical scanner.
Want me to draft these test cases?
🤖 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/repos/deps/test_imports.py` around lines 71 - 82, Extend
test_build_config_import_not_counted in test_imports.py to cover the two missed
cases from imports.py: add a generic app.config.js runtime file with a real
source import so it is counted as a shipping import, and add a Python import
fixture using comma-separated names such as import os, sys, requests to verify
_PY_IMPORT_RE and the import-site collectors handle multiple imports on one
line. Use find_import_sites and collect_import_sites to assert the new cases
behave correctly alongside the existing build-config exclusions.
There was a problem hiding this comment.
Commit 02c464f addressed this comment by adding a test that verifies comma-separated Python imports are all collected (test_py_multi_import_all_captured), which covers the _PY_IMPORT_RE gap and the import-site collector behavior. It also updates imports.py to split import a, b lines into multiple names.
There was a problem hiding this comment.
Skipped: comment is from another GitHub bot.
Correctness/safety (potential false-clears on other repo layouts; gold gate
unchanged at 128 cleared / 0 non-noise):
- imports: bucket Python imports by TOP-LEVEL module (from google.protobuf ->
'google') so build.py's lookup matches; capture multi-imports (import a, b);
count 'export ... from' barrel re-exports; revert the over-broad *.config.ts
exclusion (config files import runtime deps).
- build: build_dep_manifest never raises — a malformed lockfile no longer fails
the whole profile (which disabled the code_map/dep gates + Deep dive); pypi
import-site lookup by top-level component.
- resolver: import-site gate fails CLOSED on missing/non-list data; match pypi
names PEP-503-normalized (scanner reports 'PyYAML' vs node 'pyyaml').
- manifests: drop coupled bare 'site'/'demo' from the docs-dir set (+ document
the two-gates-share-a-taxonomy edge case); discover requirements/*.{txt,in};
scope non-dev poetry groups by name.
Reliability:
- profile_agents: run the synchronous whole-repo scan in an executor, not on
the event loop.
Cleanup:
- delete dead find_import_sites/_npm_pattern/_py_pattern (product uses
collect_import_sites); collapse duplicate _berry_name/_classic_name ->
_descriptor_name; drop the unnecessary __init__ lazy import (no cycle).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Safety/correctness:
- resolver scope gate: allowlist {dev,test,docs,build} instead of denylist
{prod,optional} — an unknown/future scope can never silently pass.
- pypi: per-directory lock precedence — a lock-less sub-project's declared deps
are no longer dropped when a sibling has a lock (was a monorepo under-connect).
- npm: _read_package_json returns None on parse failure; callers record it in
unresolved instead of silently dropping a workspace's declared deps.
- manifests: case-insensitive _SKIP_DIRS walk (consistent with imports.py).
Lint/types (ruff CI, line-length 100):
- wrap all E501 long lines; remove F401 unused import; SIM102 in _nearest_lock_dir;
merge duplicate .manifests import; DepNode.ecosystem -> Literal['npm','pypi'].
+ regression tests: allowlist unknown-scope block, pypi lock-less-subproject,
npm barrel re-exports, py dotted/multi imports, import-gate fail-closed,
pypi PEP-503 name matching.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
These two files' line-wraps were left out of the previous commit — CI ruff flagged triage_runner.py:138 and knowledge.py:70. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
What
Resolves the one triage class the resolver has always punted on: dependency findings (
locationshapepkg@version). Adds a deterministic, per-repodep_manifestprofile artifact (a lockfile-resolved dependency graph) and a pureresolve_by_dep_manifestresolver that clears a dependency finding asfalse_positiveonly when the package provably does not ship.Design + rationale: ADR-0055 (
docs/adr/0055-dependency-manifest-resolver.mdin cliff-os), superseding the 2026-06-29 DEFERRED spec.How it's safe
dep_manifestis keyed by(name, version)and computed from the lockfile graph —scopesis the union of every root that reaches a node, prod dominates. The resolver clears iff two independent gates both hold:scopes ⊆ {dev,test,docs,build}(no prod/optional) andimport_sites == [](repo-wide first-party import scan). This is strictly safer than declaration parsing:undici@6.23.0(declared prod) andundici@7.16.0(transitive) are distinct nodes; the finding's exact version is resolved.postcss@8.5.3is a declared devDependency inapps/webyet is also reachable from a prod dep — it ships. A declaration-only "declared-dev / not-declared-prod" rule false-clears it; the graph does not.A
dep_manifestclear is a pure structural clear likecode_map(lockfile facts, not a model hunch) — tier-independent under ADR-0054.Result (gold corpus)
Against the 559 previously-deferred dependency findings (
eval/corpus/resolver_precision.py depsin cliff-os): 128 cleared (24% of the 529 noise), 0 non-noise clears (the cardinal rule). The cleared set is the provably-non-shipping subset (116 dev/docs/test + 12 transitive-only); the rest ship transitively and are correctly left to reachability (Lane B). ~330 unit tests.Changes
repos/schemas.py—DepNode/DepManifest.repos/deps/— deterministic lockfile parsers (pnpm / yarn classic+berry / package-lock; uv / poetry / Pipfile / declared-only) + graph scope-propagation + first-party import scan + dist→import map + assembler.repos/repo_dir_manager.py,repos/profile_agents.py,repos/knowledge.py— register the artifact + a deterministic builder.agents/triage_dep_manifest.py+agents/triage_runner.py— the pure resolver + a gate between the code_map gate and the Deep dive.agents/triage_deep/{integration,runner}.py— threaddep_manifestinto the Deep dive so reachability (Lane B) can consume it.Notes
feat/triage-codemap-resolver(base of this PR) — merge that first, or rebase ontomain.requirements*.txt.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes