Skip to content

Enhance sorting of installable files to prioritize shallower paths in getProjectInstallable function#1428

Open
eleanorjboyd wants to merge 5 commits intomicrosoft:mainfrom
eleanorjboyd:implicit-boar
Open

Enhance sorting of installable files to prioritize shallower paths in getProjectInstallable function#1428
eleanorjboyd wants to merge 5 commits intomicrosoft:mainfrom
eleanorjboyd:implicit-boar

Conversation

@eleanorjboyd
Copy link
Copy Markdown
Member

@eleanorjboyd eleanorjboyd commented Apr 3, 2026

helps with #1429

@eleanorjboyd eleanorjboyd self-assigned this Apr 3, 2026
@eleanorjboyd eleanorjboyd added the feature-request Request for new features or functionality label Apr 3, 2026
@eleanorjboyd eleanorjboyd requested a review from Copilot April 14, 2026 20:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Improves the ordering of detected installable files so that shallower (more top-level) dependency files are prioritized, addressing sorting issues in getProjectInstallable (helps with #1429).

Changes:

  • Add depth-based sorting for matched installable URIs in getProjectInstallable.
  • Add a unit test validating shallower paths are ordered before deeper ones.
Show a summary per file
File Description
src/managers/builtin/pipUtils.ts Replaces default sort with a comparator that prioritizes shallower filesystem paths.
src/test/managers/builtin/pipUtils.unit.test.ts Adds a regression test ensuring shallower requirements files appear first.

Copilot's findings

Comments suppressed due to low confidence (1)

src/managers/builtin/pipUtils.ts:337

  • The new depth sort implies the returned installables order matters, but this block mutates installable from concurrent async tasks. If any .toml entries are present, await tomlParse(...) can cause later push(...) calls to occur out of the sorted order, making the final installables ordering nondeterministic. To keep ordering deterministic, build per-URI results (e.g., return an Installable[] from each mapper) and then flatten in filtered order, or process filtered sequentially when ordering is required.
            await Promise.all(
                filtered.map(async (uri) => {
                    if (uri.fsPath.endsWith('.toml')) {
                        const toml = await tomlParse(uri.fsPath);

                        // Validate pyproject.toml
                        if (!validationError) {
                            const error = validatePyprojectToml(toml);
                            if (error) {
                                validationError = {
                                    message: error,
                                    fileUri: uri,
                                };
                            }
                        }

                        installable.push(...getTomlInstallable(toml, uri));
                    } else {
                        const name = path.basename(uri.fsPath);
                        installable.push({
                            name,
                            uri,
                            displayName: name,
                            group: 'Requirements',
                            args: ['-r', uri.fsPath],
                        });
                    }
                }),
            );
  • Files reviewed: 2/2 changed files
  • Comments generated: 2

Comment thread src/managers/builtin/pipUtils.ts
Comment thread src/test/managers/builtin/pipUtils.unit.test.ts
@eleanorjboyd eleanorjboyd marked this pull request as ready for review May 6, 2026 17:54
@StellaHuang95
Copy link
Copy Markdown
Contributor

looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature-request Request for new features or functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants