Skip to content

Commit ec7e629

Browse files
committed
feat(python): skip-if-listed and search-first logic for package management
- Updated PipEnvironmentProvider and PixiEnvironmentProvider to skip installation of already listed packages. - Introduced GetListJsonAsync method for both providers to retrieve installed package states. - Enhanced package installation logic to prioritize conda and PyPI based on availability. - Added tests to verify the new behavior for package management and ensure correct handling of installed states. - Updated documentation to reflect changes in decision-making for package installation strategies.
1 parent 87ff7e2 commit ec7e629

15 files changed

Lines changed: 561 additions & 138 deletions

docs/agents/known-test-gaps.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ Thin unit layer (`tests/DevTools.Execution.Tests`, ~23 cases).
7171

7272
### Well covered
7373

74-
- **Python environment** — pip/pixi path resolution, env probing
74+
- **Python environment** — pip fixtures; Pixi helper unit tests (`PartitionByAvailability`,
75+
`ExtractPackageName`); Parser installed-state via list JSON; opt-in smoke
76+
(`RUN_PIXI_SMOKE=1` → setup + Python.NET import)
7577
- **Execution guard**`ExecutionGuardContext` ambient mode / rollback summary
7678

7779
### Gaps (low automated coverage)
@@ -81,6 +83,7 @@ Thin unit layer (`tests/DevTools.Execution.Tests`, ~23 cases).
8183
- **Host threading**`IHostContextExecutor`, main-thread marshaling for API calls
8284
- **MCP dispatch from execution**`McpPrimitiveDispatcher`, `ToolInvoke.py` payload path
8385
- **Built-in tools** — open document, registry providers
86+
- **Pixi AppData ensure/install** — opt-in smoke only; no CI coverage of cold wipe or search-first add
8487

8588
`tests/RevitDevTool.PyServer.Tests/` — small Python parser check only.
8689

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# 0014 Skip-If-Listed + Search-First (Pixi/Pip)
2+
3+
Date: 2026-08-06
4+
Updated: 2026-08-06
5+
6+
## Status
7+
8+
Accepted
9+
10+
## Context
11+
12+
Install/ensure used **try-first** (`pixi add` / `pip install` even when packages
13+
were already present; Pixi also failed-then-retried channel selection). Prefer
14+
**skip when listed**, then classify missing packages once (Pixi).
15+
16+
This is a small ensure/install improvement. Warm host open still depends on
17+
`IsEnvironmentReady()` (home set + `python.exe`); it is not a startup rewrite.
18+
19+
## Decision
20+
21+
1. **Skip if listed (Pixi and Pip)**`GetListJsonAsync` (`pixi list --json` /
22+
`pip list --format=json`) supplies installed names before require-ensure,
23+
`InstallPackagesAsync`, and PEP 723 resolve (`PythonDepsManager` → Parser).
24+
2. **Search-first (Pixi only)** — for missing specs: `pixi search --limit 1`
25+
(exit code) → at most one conda batch and one PyPI batch. Not fail-then-retry
26+
as primary. Pip is PyPI-only: list-skip then `pip install`.
27+
3. **Provider shape**`PyEnvironmentProvider` owns process-scoped
28+
`PythonHome` via abstract `ResolvePythonHomeAsync` + `EnsurePythonHomeAsync`
29+
(assign once). `IsEnvironmentReady()` is read-only. `GetListJsonAsync` is
30+
abstract on the base (no `Backend` switch in callers).
31+
32+
## Non-goals
33+
34+
- Claiming CLI version bumps as a latency win.
35+
- Pixi shell-hook / fake `CONDA_PREFIX` activation in the host process.
36+
37+
## Consequences
38+
39+
Ensure/install avoid redundant adds and systematic failed solves.
40+
`PixiPackageHelper` may still read list `kind` for remove/update.

docs/decisions/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,4 @@ here.
2222
| [0010](0010-daemon-sole-mcp-host.md) | Daemon is sole MCP host | Accepted |
2323
| [0011](0011-hybrid-repository-harness-layout.md) | Hybrid repository-harness docs layout | Accepted |
2424
| [0012](0012-host-mcp-spec-engine.md) | Host MCP spec engine (no SDK on host) | Accepted |
25+
| [0014](0014-pep723-skip-if-listed-search-first.md) | Skip-if-listed + search-first (Pixi/Pip) | Accepted |

docs/plans/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ decision into `docs/decisions/`; keep task-local choices in the plan.
3030

3131
| Plan | Status |
3232
|------|--------|
33-
| [2026-07-27-mcp-dynamic-discovery-and-python-toolset.md](active/2026-07-27-mcp-dynamic-discovery-and-python-toolset.md) | Active — Python alias fix + hybrid search + round-trip cuts |
34-
| [2026-07-26-mcp-agent-efficiency.md](active/2026-07-26-mcp-agent-efficiency.md) | Active — Phase 1 MCP re-measure / Phase 3 optional pending |
33+
| [2026-08-02-mrtr-implementation.md](active/2026-08-02-mrtr-implementation.md) | Active — G1 closed; G3/G4 open |
3534

3635
## Recently Completed
3736

3837
| Plan | Completed |
3938
|------|-----------|
39+
| [2026-08-06-pixi-skip-if-listed.md](completed/2026-08-06-pixi-skip-if-listed.md) | 2026-08-06 |
4040
| [2026-07-25-mcp-call-observability.md](completed/2026-07-25-mcp-call-observability.md) | 2026-07-25 |
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# Execution Plan: Skip-if-listed + Pixi search-first
2+
3+
Date: 2026-08-06
4+
Completed: 2026-08-06
5+
6+
## Status
7+
8+
Completed
9+
10+
## Outcome
11+
12+
Land [0014](../../decisions/0014-pep723-skip-if-listed-search-first.md): replace
13+
try-first with skip-if-listed (both backends) and Pixi search-first. Pin Pixi
14+
**0.76.1**. Small ensure/install change — not a host-startup benchmark.
15+
16+
## Done (matches code)
17+
18+
- [x] `PythonInstaller.PixiVersion` = `0.76.1`
19+
- [x] `PyEnvironmentProvider`: `ResolvePythonHomeAsync` / `EnsurePythonHomeAsync`,
20+
read-only `IsEnvironmentReady`, abstract `GetListJsonAsync`, shared
21+
`GetInstalledNamesAsync`
22+
- [x] Pixi: list → skip; missing → search → add batches
23+
- [x] Pip: list → skip before `pip install` (require ensure + `InstallPackagesAsync`)
24+
- [x] `PythonDepsManager`: `provider.GetListJsonAsync` → Parser stdin (no Backend switch)
25+
- [x] No shell-hook / fake activation
26+
- [x] Tests: `PixiEnvironmentProviderTests`; opt-in `PixiEnvironmentSmokeTests`
27+
(`RUN_PIXI_SMOKE=1`)
28+
29+
## Result
30+
31+
Ensure/install paths skip when already listed. Warm open still short-circuits
32+
when `PythonHome` is set and `python.exe` exists after setup in-process.

source/DevTools.Execution/Providers/Python/PipEnvironmentProvider.cs

Lines changed: 49 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,19 @@ namespace DevTools.Execution.Providers.Python;
1313
/// where pixi.exe cannot execute due to security policies.
1414
/// Discovers the CPython distribution shipped with pyRevit (cengines directory),
1515
/// bootstraps pip, and uses <c>python.exe -m pip</c> for package management.
16+
/// Policy: skip if listed; otherwise pip install (single channel — no search-first).
1617
/// </summary>
1718
public sealed class PipEnvironmentProvider(ILogger<PipEnvironmentProvider> logger) : PyEnvironmentProvider
1819
{
1920
public override PythonBackend Backend => PythonBackend.Pip;
2021

22+
protected override Task<string> ResolvePythonHomeAsync()
23+
=> DiscoverPyRevitAsync();
24+
2125
public override async Task SetupEnvironmentAsync()
2226
{
23-
PythonHomePath = await DiscoverPyRevitAsync().ConfigureAwait(false);
24-
RemovePthFile(PythonHomePath);
27+
await EnsurePythonHomeAsync().ConfigureAwait(false);
28+
RemovePthFile(PythonHome);
2529

2630
if (!await IsPipAvailableAsync().ConfigureAwait(false))
2731
await BootstrapPipAsync().ConfigureAwait(false);
@@ -105,12 +109,19 @@ private static async Task<List<string>> GetAttachedClonePathsAsync()
105109

106110
private async Task EnsureRequirePackagesAsync()
107111
{
108-
// pip install is idempotent with version constraints:
109-
// no-op if constraint is already satisfied, upgrades if version falls outside range.
110-
var specs = RequirePackages.Values.ToList();
112+
var installed = await GetInstalledNamesAsync().ConfigureAwait(false);
113+
var missing = RequirePackages.Values
114+
.Where(spec => !installed.Contains(ExtractPackageName(spec)))
115+
.ToList();
116+
117+
if (missing.Count == 0)
118+
{
119+
logger.ZLogDebug($"[Pip] Require packages already installed — skipping pip install.");
120+
return;
121+
}
111122

112123
var args = new List<string> { "-m", "pip", "install", "--prefer-binary", "--no-warn-script-location" };
113-
args.AddRange(specs);
124+
args.AddRange(missing);
114125

115126
var result = await Cli.Wrap(PythonExe)
116127
.WithArguments(args)
@@ -121,21 +132,29 @@ private async Task EnsureRequirePackagesAsync()
121132
.ExecuteAsync().ConfigureAwait(false);
122133

123134
if (result.ExitCode != 0)
124-
throw new InvalidOperationException($"Failed to verify required packages: {string.Join(", ", specs)}");
135+
throw new InvalidOperationException($"Failed to verify required packages: {string.Join(", ", missing)}");
125136
}
126137

127138
public override async Task InstallPackagesAsync(
128139
IEnumerable<string> packages,
129140
IProgress<string> progress,
130141
CancellationToken cancellationToken)
131142
{
132-
var list = packages.ToList();
133-
if (list.Count == 0) return;
143+
var requested = packages.ToList();
144+
if (requested.Count == 0) return;
134145

135-
progress.Report($"Installing {list.Count} package(s) via pip: {string.Join(", ", list)}");
146+
var installed = await GetInstalledNamesAsync(cancellationToken).ConfigureAwait(false);
147+
var missing = requested.Where(spec => !installed.Contains(ExtractPackageName(spec))).ToList();
148+
if (missing.Count == 0)
149+
{
150+
progress.Report("All requested packages already installed.");
151+
return;
152+
}
153+
154+
progress.Report($"Installing {missing.Count} package(s) via pip: {string.Join(", ", missing)}");
136155

137156
var (succeeded, failed) = await TryPipInstallBatchAsync(
138-
list, progress, cancellationToken).ConfigureAwait(false);
157+
missing, progress, cancellationToken).ConfigureAwait(false);
139158

140159
if (succeeded.Count > 0 && failed.Count > 0)
141160
progress.Report($"pip: {string.Join(", ", succeeded)}");
@@ -146,7 +165,25 @@ public override async Task InstallPackagesAsync(
146165
$"Failed to install the following package(s): {string.Join(", ", failed)}");
147166
}
148167

149-
progress.Report($"All {list.Count} package(s) installed via pip.");
168+
progress.Report($"All {requested.Count} package(s) processed via pip.");
169+
}
170+
171+
/// <inheritdoc />
172+
public override async Task<string> GetListJsonAsync(CancellationToken cancellationToken = default)
173+
{
174+
if (!IsEnvironmentReady())
175+
return string.Empty;
176+
177+
var stdout = new StringBuilder();
178+
var result = await Cli.Wrap(PythonExe)
179+
.WithArguments(["-m", "pip", "list", "--format=json"])
180+
.WithWorkingDirectory(PythonHome)
181+
.WithStandardOutputPipe(PipeTarget.ToStringBuilder(stdout))
182+
.WithValidation(CommandResultValidation.None)
183+
.ExecuteAsync(cancellationToken)
184+
.ConfigureAwait(false);
185+
186+
return result.ExitCode == 0 ? stdout.ToString().Trim() : string.Empty;
150187
}
151188

152189
private void RemovePthFile(string targetDir)

0 commit comments

Comments
 (0)