Resolve requirement names from distribution metadata - #533
Conversation
📝 WalkthroughWalkthroughRequirements generation now uses configured Python environments and distribution metadata to resolve imported modules into PEP 508 requirements. Configuration, CLI and environment overrides, ownership classification, bundled Python metadata lookup, ambiguity handling, and integration tests were added. ChangesPython requirement resolution
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Orchestrator
participant RequirementResolver
participant PythonHelper
participant DistributionMetadata
CLI->>Orchestrator: generate requirements
Orchestrator->>RequirementResolver: resolve imported modules
RequirementResolver->>PythonHelper: send imports and metadata paths
PythonHelper->>DistributionMetadata: inspect installed distributions
DistributionMetadata-->>PythonHelper: return scored candidates
PythonHelper-->>RequirementResolver: return resolutions
RequirementResolver-->>Orchestrator: return PEP 508 requirements
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
📊 Ecosystem Test Results📋 Test StatusTest Summary:
📈 Benchmark Results📊 View detailed benchmark report 📦 Package Bundling Metrics
Benchmark metrics are tracked via Bencher.dev 📊 View detailed performance trends and comparisons on the Bencher dashboard. Generated by ecosystem-tests workflow |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the accuracy of requirement generation by moving away from simple import-to-package name assumptions. By leveraging actual Python environment metadata, the tool can now correctly identify distributions for packages with non-obvious names, such as scikit-learn. The changes include a new resolution module, enhanced configuration options for interpreter selection, and better handling of ambiguous namespace providers. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Bencher Report| Branch | codex/resolve-requirement-distributions |
| Testbed | ubuntu-latest |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result nanoseconds (ns) (Result Δ%) | Upper Boundary nanoseconds (ns) (Limit %) |
|---|---|---|---|
| build_dependency_graph | 📈 view plot 🚷 view threshold | 626.80 ns(-90.14%)Baseline: 6,356.00 ns | 15,194.65 ns (4.13%) |
| bundle_simple_project | 📈 view plot 🚷 view threshold | 1,429,700.00 ns(-53.29%)Baseline: 3,060,777.81 ns | 15,245,117.38 ns (9.38%) |
| resolve_module_path | 📈 view plot 🚷 view threshold | 115.20 ns(+3.63%)Baseline: 111.16 ns | 144.18 ns (79.90%) |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a requirement resolution mechanism to map Python imports to installed distribution packages (PEP 508 requirements) using a new RequirementResolver and a Python helper script. The review feedback highlights several critical improvements: resolving potential encoding issues on Windows by using UTF-8 buffers for standard I/O, supporting absolute paths for editable installs, defensively catching general exceptions for malformed metadata, and ensuring case-insensitive matching for Core Metadata headers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if path == prefix + ".py" or path == prefix + "/__init__.py": | ||
| return 4000 + depth, "installed file" | ||
|
|
||
| for suffix in importlib.machinery.EXTENSION_SUFFIXES: | ||
| if path == prefix + suffix or path == prefix + "/__init__" + suffix: | ||
| return 4000 + depth, "installed extension" | ||
|
|
||
| if path.startswith(prefix + "/"): | ||
| return 3000 + depth, "installed namespace descendant" |
There was a problem hiding this comment.
To support editable installs and other non-standard virtualenv layouts where PackagePath objects can return absolute paths, we should make file_score robust against absolute paths by checking if the path ends with the expected relative path preceded by a path separator.
| if path == prefix + ".py" or path == prefix + "/__init__.py": | |
| return 4000 + depth, "installed file" | |
| for suffix in importlib.machinery.EXTENSION_SUFFIXES: | |
| if path == prefix + suffix or path == prefix + "/__init__" + suffix: | |
| return 4000 + depth, "installed extension" | |
| if path.startswith(prefix + "/"): | |
| return 3000 + depth, "installed namespace descendant" | |
| if path == prefix + ".py" or path.endswith("/" + prefix + ".py") or path == prefix + "/__init__.py" or path.endswith("/" + prefix + "/__init__.py"): | |
| return 4000 + depth, "installed file" | |
| for suffix in importlib.machinery.EXTENSION_SUFFIXES: | |
| if path == prefix + suffix or path.endswith("/" + prefix + suffix) or path == prefix + "/__init__" + suffix or path.endswith("/" + prefix + "/__init__" + suffix): | |
| return 4000 + depth, "installed extension" | |
| if path.startswith(prefix + "/") or ("/" + prefix + "/") in path: |
There was a problem hiding this comment.
Addressed in d6f5d2a. Absolute package paths are relativized against normalized import roots before root-anchored matching. Coverage verifies an editable absolute path and rejects nested paths such as pandera/api/polars; full validation: nextest 199/199, Insta 380/380, strict Clippy, fmt, and Ruff.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60724723c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
📊 Source Code Metrics (this PR vs
|
| File | Cyclomatic | Cognitive | Functions | LLOC | MI |
|---|---|---|---|---|---|
| crates/cribo/src/resolver.rs | 435 (main: 413) 🔴 | 372 (main: 374) 🟢 | 90 (main: 84) 🔴 | 789 (main: 762) 🔴 | 0 ⚪ |
| crates/cribo/src/orchestrator.rs | 240 (main: 236) 🔴 | 259 (main: 256) 🔴 | 36 ⚪ | 487 (main: 485) 🔴 | 0 ⚪ |
| crates/cribo/src/config.rs | 82 (main: 71) 🔴 | 48 (main: 42) 🔴 | 19 (main: 14) 🔴 | 131 (main: 109) 🔴 | 0.33 (main: 4.56) 🔴 |
| crates/cribo/src/requirement_resolver.rs | 100 🆕 | 75 🆕 | 18 🆕 | 148 🆕 | 0 🆕 |
| crates/cribo/src/requirement_resolver.py | 81 🆕 | 76 🆕 | 16 🆕 | 140 🆕 | 6.79 🆕 |
| crates/cribo/src/main.rs | 18 (main: 17) 🔴 | 14 (main: 13) 🔴 | 1 ⚪ | 28 (main: 26) 🔴 | 25.01 (main: 25.94) 🔴 |
| crates/cribo/src/lib.rs | 1 ⚪ | 0 ⚪ | 0 ⚪ | 0 ⚪ | 45.68 (main: 46.09) 🔴 |
Generated by mehen v1.3.0 — the code quality watcher.
There was a problem hiding this comment.
Pull request overview
This PR improves --emit-requirements by resolving requirement (distribution) names from the selected Python environment’s installed distribution metadata, decoupling requirement inference from module classification, and adding configuration/CLI/env plumbing to control which interpreter is inspected.
Changes:
- Introduce a dedicated requirement resolver (Rust + embedded Python) that maps imports to normalized distribution names using Core Metadata
Import-Name/Import-Namespace, installed files, andtop_level.txt, with actionable ambiguity errors. - Add interpreter selection via
--python,CRIBO_PYTHON, and[requirements]config (including longest-prefixmodule-mapoverrides). - Add end-to-end fixtures + snapshot updates covering distribution-name mismatches and ambiguous namespace providers.
Reviewed changes
Copilot reviewed 32 out of 33 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents --python and [requirements] configuration examples. |
| python/cribo/main.py | Ensures python -m cribo defaults CRIBO_PYTHON to the invoking interpreter. |
| docs/resolution.md | Updates resolution docs to reflect separated requirement inference and new mapping rules. |
| docs/environment_variables.md | Documents CRIBO_PYTHON and its intended behavior. |
| cribo.toml | Adds [requirements] config stub and module-map default. |
| crates/cribo/tests/snapshots/ruff_lint_results@requirements_distribution_mapping.snap | Snapshot for ruff lint results for new fixture. |
| crates/cribo/tests/snapshots/requirements@try_except_function_export.snap | Updates requirements snapshot to reflect normalized fallback naming. |
| crates/cribo/tests/snapshots/requirements@requirements_distribution_mapping.snap | New snapshot verifying mapped distribution names from metadata. |
| crates/cribo/tests/snapshots/requirements@conditional_imports.snap | Updates snapshot to reflect normalized distribution naming. |
| crates/cribo/tests/snapshots/execution_results@requirements_distribution_mapping.snap | New execution snapshot for the distribution-mapping fixture. |
| crates/cribo/tests/snapshots/bundling_error@xfail_ambiguous_requirement_namespace.snap | New snapshot validating actionable error on ambiguous providers. |
| crates/cribo/tests/snapshots/bundled_code@requirements_distribution_mapping.snap | New bundled-code snapshot for the distribution-mapping fixture. |
| crates/cribo/tests/fixtures/xfail_ambiguous_requirement_namespace/shared/beta.py | Fixture module for ambiguity scenario. |
| crates/cribo/tests/fixtures/xfail_ambiguous_requirement_namespace/shared/alpha.py | Fixture module for ambiguity scenario. |
| crates/cribo/tests/fixtures/xfail_ambiguous_requirement_namespace/main.py | Fixture entrypoint that triggers ambiguous namespace mapping. |
| crates/cribo/tests/fixtures/xfail_ambiguous_requirement_namespace/beta_distribution-1.0.dist-info/METADATA | Fixture dist metadata (Beta) including Import-* fields. |
| crates/cribo/tests/fixtures/xfail_ambiguous_requirement_namespace/alpha_distribution-1.0.dist-info/METADATA | Fixture dist metadata (Alpha) including Import-* fields. |
| crates/cribo/tests/fixtures/requirements_distribution_mapping/shared/beta/init.py | Fixture module for mapping scenario. |
| crates/cribo/tests/fixtures/requirements_distribution_mapping/shared/alpha/init.py | Fixture module for mapping scenario. |
| crates/cribo/tests/fixtures/requirements_distribution_mapping/main.py | Fixture entrypoint verifying mapped requirements. |
| crates/cribo/tests/fixtures/requirements_distribution_mapping/beta_distribution-1.0.dist-info/RECORD | Fixture installed-file evidence (RECORD) for Beta distribution. |
| crates/cribo/tests/fixtures/requirements_distribution_mapping/beta_distribution-1.0.dist-info/METADATA | Fixture metadata for Beta distribution (older metadata version). |
| crates/cribo/tests/fixtures/requirements_distribution_mapping/alpha_distribution-1.0.dist-info/METADATA | Fixture metadata for Alpha distribution (with Import-* fields). |
| crates/cribo/src/resolver.rs | Removes requirement from import classification and adds metadata-based ownership check. |
| crates/cribo/src/requirement_resolver.rs | New Rust requirement resolver orchestrating metadata queries and selection logic. |
| crates/cribo/src/requirement_resolver.py | Embedded Python metadata query implementation using importlib.metadata. |
| crates/cribo/src/orchestrator.rs | Routes requirements generation through the new resolver and propagates errors. |
| crates/cribo/src/main.rs | Adds --python CLI flag and wires it into config. |
| crates/cribo/src/lib.rs | Exposes the new requirement_resolver module internally. |
| crates/cribo/src/config.rs | Adds [requirements] configuration + CRIBO_PYTHON env support. |
| crates/cribo/Cargo.toml | Adds serde_json dependency usage for the crate. |
| Cargo.toml | Adds workspace dependency on serde_json. |
| Cargo.lock | Locks serde_json addition. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tinovyatkin
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6f5d2acac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@crates/cribo/src/config.rs`:
- Around line 46-54: Update the Debug behavior for RequirementsConfig and its
use within Config so module_map values are not emitted in Config debug logs,
while preserving useful non-sensitive configuration details. Prefer a custom
redacted Debug implementation that masks the complete PEP 508 mapping contents,
and ensure the existing Config logging path uses this redacted representation.
In `@crates/cribo/src/requirement_resolver.py`:
- Around line 65-134: Build a reusable distribution-ownership index so installed
metadata is scanned once rather than once per import. In
crates/cribo/src/requirement_resolver.py:65-134, refactor
distribution_candidates and main to index metadata headers, importable files,
and top-level names before resolving imports. In
crates/cribo/src/resolver.rs:1276-1323, cache the corresponding ownership data
by search root and reuse it instead of rereading METADATA and RECORD for each
lookup.
- Around line 72-93: Update the requirement resolution logic around the
Import-Name and Import-Namespace metadata loops to detect overlapping declared
names before calling add_candidate. If the same name appears in both fields,
fail fast rather than scoring either candidate; preserve the existing scoring
behavior when the fields are non-overlapping.
- Around line 67-93: Guard access to distribution.metadata in the distribution
iteration so metadata-read exceptions are handled like the existing files and
top_level.txt lookups. Skip the affected distribution and continue resolving
other candidates, while preserving the Import-Name and Import-Namespace
processing for successfully read metadata.
In `@crates/cribo/src/requirement_resolver.rs`:
- Around line 43-283: Document every newly introduced Rust function with
appropriate doc comments: add method and helper documentation throughout
RequirementResolver in crates/cribo/src/requirement_resolver.rs (lines 43-283),
document the environment-path parsing function in crates/cribo/src/config.rs
(lines 233-239), the classification constructor in crates/cribo/src/resolver.rs
(line 303), and metadata-header matching functions in
crates/cribo/src/resolver.rs (lines 1325-1341).
In `@python/cribo/__main__.py`:
- Around line 16-19: Update the environment setup in the module entrypoint to
replace CRIBO_PYTHON when it is unset or empty, using sys.executable in both
cases. Preserve any non-empty CRIBO_PYTHON value before invoking subprocess.run.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 830a831c-b73c-43a8-b6d5-d857a58a19be
⛔ Files ignored due to path filters (21)
Cargo.lockis excluded by!**/*.lockcrates/cribo/tests/fixtures/requirements_distribution_mapping/alpha_distribution-1.0.dist-info/METADATAis excluded by!**/tests/fixtures/**crates/cribo/tests/fixtures/requirements_distribution_mapping/beta_distribution-1.0.dist-info/METADATAis excluded by!**/tests/fixtures/**crates/cribo/tests/fixtures/requirements_distribution_mapping/beta_distribution-1.0.dist-info/RECORDis excluded by!**/tests/fixtures/**crates/cribo/tests/fixtures/requirements_distribution_mapping/main.pyis excluded by!**/tests/fixtures/**crates/cribo/tests/fixtures/requirements_distribution_mapping/shared/alpha/__init__.pyis excluded by!**/tests/fixtures/**crates/cribo/tests/fixtures/requirements_distribution_mapping/shared/beta/__init__.pyis excluded by!**/tests/fixtures/**crates/cribo/tests/fixtures/xfail_ambiguous_requirement_namespace/alpha_distribution-1.0.dist-info/METADATAis excluded by!**/tests/fixtures/**crates/cribo/tests/fixtures/xfail_ambiguous_requirement_namespace/beta_distribution-1.0.dist-info/METADATAis excluded by!**/tests/fixtures/**crates/cribo/tests/fixtures/xfail_ambiguous_requirement_namespace/main.pyis excluded by!**/tests/fixtures/**crates/cribo/tests/fixtures/xfail_ambiguous_requirement_namespace/shared/alpha.pyis excluded by!**/tests/fixtures/**crates/cribo/tests/fixtures/xfail_ambiguous_requirement_namespace/shared/beta.pyis excluded by!**/tests/fixtures/**crates/cribo/tests/snapshots/bundled_code@requirements_distribution_mapping.snapis excluded by!**/*.snapcrates/cribo/tests/snapshots/bundling_error@xfail_ambiguous_requirement_namespace.snapis excluded by!**/*.snapcrates/cribo/tests/snapshots/execution_results@requirements_distribution_mapping.snapis excluded by!**/*.snapcrates/cribo/tests/snapshots/requirements@conditional_imports.snapis excluded by!**/*.snapcrates/cribo/tests/snapshots/requirements@requirements_distribution_mapping.snapis excluded by!**/*.snapcrates/cribo/tests/snapshots/requirements@try_except_function_export.snapis excluded by!**/*.snapcrates/cribo/tests/snapshots/ruff_lint_results@requirements_distribution_mapping.snapis excluded by!**/*.snapdocs/environment_variables.mdis excluded by!**/docs/**docs/resolution.mdis excluded by!**/docs/**
📒 Files selected for processing (12)
Cargo.tomlREADME.mdcrates/cribo/Cargo.tomlcrates/cribo/src/config.rscrates/cribo/src/lib.rscrates/cribo/src/main.rscrates/cribo/src/orchestrator.rscrates/cribo/src/requirement_resolver.pycrates/cribo/src/requirement_resolver.rscrates/cribo/src/resolver.rscribo.tomlpython/cribo/__main__.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 119ec49343
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79163ee2de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/cribo/src/requirement_resolver.rs (1)
304-315: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSearch every metadata path when auto-detecting Python.
Using only the first path misses a CWD fallback virtualenv when the entry file is external, despite its site-packages being included later. The helper then runs under PATH Python, adding unrelated
sys.pathproviders and interpreter-specific extension suffixes. Iterate all metadata paths and cover the external-entry layout without an explicit--python.Proposed direction
- let first_path = self.metadata_paths.first()?; - for ancestor in first_path.ancestors() { - for environment_name in AUTO_DETECTED_VIRTUALENV_NAMES { - let candidate = Self::environment_python(&ancestor.join(environment_name)); - if candidate.is_file() { - return Some(candidate); + for metadata_path in &self.metadata_paths { + for ancestor in metadata_path.ancestors() { + for environment_name in AUTO_DETECTED_VIRTUALENV_NAMES { + let candidate = Self::environment_python(&ancestor.join(environment_name)); + if candidate.is_file() { + return Some(candidate); + } } } }Also applies to: 388-408
🤖 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 `@crates/cribo/src/requirement_resolver.rs` around lines 304 - 315, Update auto_detected_python to iterate through every path in self.metadata_paths rather than only self.metadata_paths.first(), checking each path’s ancestors for the conventional virtualenv names. Preserve the existing candidate validation and return the first matching interpreter, including virtualenvs associated with external-entry and CWD metadata paths.crates/cribo/src/resolver.rs (1)
1367-1387: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winIndex
top_level.txtbefore classifying resolved imports as first-party.This index ignores the legacy source supported by
requirement_resolver.py. A PYTHONPATH distribution with onlytop_level.txtownership is classifiedFirstParty, sogenerate_requirementsnever sends it to metadata resolution. Readtop_level.txthere as well and add a regression for that metadata shape.🤖 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 `@crates/cribo/src/resolver.rs` around lines 1367 - 1387, Extend the distribution indexing flow around `index_distribution_metadata` to read each `.dist-info` directory’s `top_level.txt` and add its entries to the same ownership index before resolved imports are classified. Preserve the existing `METADATA` and `RECORD` handling, and add a regression covering a PYTHONPATH distribution whose ownership is provided only by `top_level.txt`.crates/cribo/tests/test_cli_stdout.rs (1)
21-104: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftMove these requirement cases into the generic snapshot framework.
Create fixture directories with
main.pyand extend the generic harness where environment customization is needed, rather than adding bespoke CLI setup and assertions here.As per coding guidelines, “Use the generic snapshot testing framework located at
crates/cribo/tests/test_bundling_snapshots.rsas the primary testing approach for bundling features; create fixture directories withmain.pyfiles and run usingINSTA_GLOB_FILTERenvironment variable.”Also applies to: 271-362
🤖 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 `@crates/cribo/tests/test_cli_stdout.rs` around lines 21 - 104, Move the requirement-generation cases from the bespoke helpers and assertions in this test module into the generic snapshot framework in test_bundling_snapshots.rs. Create fixture directories containing main.py for each case, extend the generic harness to support the required environment customization, and run the cases through the INSTA_GLOB_FILTER-based snapshot flow. Remove the now-unneeded helpers write_test_distribution, cribo_command, run_cribo, run_requirement_cribo, and assert_requirement_output.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@crates/cribo/src/requirement_resolver.rs`:
- Around line 304-315: Update auto_detected_python to iterate through every path
in self.metadata_paths rather than only self.metadata_paths.first(), checking
each path’s ancestors for the conventional virtualenv names. Preserve the
existing candidate validation and return the first matching interpreter,
including virtualenvs associated with external-entry and CWD metadata paths.
In `@crates/cribo/src/resolver.rs`:
- Around line 1367-1387: Extend the distribution indexing flow around
`index_distribution_metadata` to read each `.dist-info` directory’s
`top_level.txt` and add its entries to the same ownership index before resolved
imports are classified. Preserve the existing `METADATA` and `RECORD` handling,
and add a regression covering a PYTHONPATH distribution whose ownership is
provided only by `top_level.txt`.
In `@crates/cribo/tests/test_cli_stdout.rs`:
- Around line 21-104: Move the requirement-generation cases from the bespoke
helpers and assertions in this test module into the generic snapshot framework
in test_bundling_snapshots.rs. Create fixture directories containing main.py for
each case, extend the generic harness to support the required environment
customization, and run the cases through the INSTA_GLOB_FILTER-based snapshot
flow. Remove the now-unneeded helpers write_test_distribution, cribo_command,
run_cribo, run_requirement_cribo, and assert_requirement_output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f75a7197-a6aa-415c-b61f-6424f15d890e
📒 Files selected for processing (7)
crates/cribo/src/config.rscrates/cribo/src/orchestrator.rscrates/cribo/src/requirement_resolver.pycrates/cribo/src/requirement_resolver.rscrates/cribo/src/resolver.rscrates/cribo/tests/test_cli_stdout.rspython/cribo/__main__.py
Summary
Import-Name/Import-Namespace, installed-file records, and legacytop_level.txt--python,CRIBO_PYTHON, and[requirements]configuration with longest-prefixmodule-mapoverridesWhy
Requirements generation previously assumed that a top-level import name was also the installable distribution name. That produces incorrect output for packages such as
sklearn/scikit-learn, and filesystem scan order cannot safely resolve namespaces supplied by multiple distributions.The new resolver uses metadata from the environment that will run the bundle, while retaining explicit configuration for missing or ambiguous metadata.
Closes #141
User impact
--emit-requirementsnow emits normalized distribution names when installed metadata identifies them. Users can select an interpreter explicitly with--python,CRIBO_PYTHON, orrequirements.python;python -m criboautomatically supplies its own interpreter. Ambiguous providers fail with guidance to configurerequirements.module-map, while unmapped imports retain a valid normalized fallback.Validation
cargo nextest run --workspace(194 passed)cargo insta test --all-features --check --unreferenced reject(370 passed; no unreferenced snapshots)cargo clippy --workspace --all-targets --all-features --locked -- -D warningscargo fmt --all -- --checkuvx ruff check python/cribo/__main__.py crates/cribo/src/requirement_resolver.pySummary by CodeRabbit
New Features
--pythonandCRIBO_PYTHONoptions for selecting the Python interpreter.Documentation
Bug Fixes