Skip to content

Commit 4b7a780

Browse files
fix: deduplicate Windows Conda casing aliases (#519)
## Summary - preserve the canonical executable path on cache hits - keep caller-facing aliases valid in the current working context - invalidate stale in-memory entries when tracked executables disappear - deduplicate Windows Conda casing aliases while preserving on-disk spelling - version performance inventory semantics so the intentional v1-to-v2 deduplication transition is explicit and same-schema count mismatches remain blocking Fixes #518 Related: microsoft/vscode-python-environments#1703 ## Validation - `cargo test -p pet-conda --test environment_locations_test` - `cargo test --features ci-perf --test e2e_performance test_performance_summary --no-run` - `python -B -m unittest discover -s scripts/tests -p 'test_*.py' -v` (47 passed) - `./scripts/rust-precommit.ps1` - replayed the failed Windows snapshot against its exact base: legacy comparison fails at 8/1 vs 10/2; inventory schema v2 passes with all latency metrics within budget --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent ab3d45c commit 4b7a780

8 files changed

Lines changed: 157 additions & 16 deletions

File tree

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/pet-conda/Cargo.toml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,8 @@ env_logger = "0.10.2"
2121
yaml-rust2 = "0.8.1"
2222
rayon = "1.11.0"
2323

24+
[dev-dependencies]
25+
tempfile = "3.13"
26+
2427
[features]
2528
ci = []

‎crates/pet-conda/src/environment_locations.rs‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,25 @@ pub fn get_conda_envs_from_environment_txt(env_vars: &EnvVariables) -> Vec<PathB
319319
envs
320320
}
321321

322+
#[cfg(windows)]
323+
fn restore_existing_leaf_case(path: PathBuf) -> PathBuf {
324+
let Some(parent) = path.parent() else {
325+
return path;
326+
};
327+
let Some(file_name) = path.file_name() else {
328+
return path;
329+
};
330+
let Ok(entries) = fs::read_dir(parent) else {
331+
return path;
332+
};
333+
334+
entries
335+
.filter_map(Result::ok)
336+
.find(|entry| entry.file_name().eq_ignore_ascii_case(file_name))
337+
.map(|entry| entry.path())
338+
.unwrap_or(path)
339+
}
340+
322341
#[cfg(windows)]
323342
pub fn get_known_conda_install_locations(
324343
env_vars: &EnvVariables,
@@ -416,15 +435,6 @@ pub fn get_known_conda_install_locations(
416435
.join("conda"),
417436
);
418437
}
419-
known_paths.sort();
420-
known_paths.dedup();
421-
// Ensure the casing of the paths are correct.
422-
// Its possible the actual path is in a different case.
423-
// E.g. instead of C:\username\miniconda it might bt C:\username\Miniconda
424-
// We use lower cases above, but it could be in any case on disc.
425-
// We do not want to have duplicates in different cases.
426-
// & we'd like to preserve the case of the original path as on disc.
427-
known_paths = known_paths.iter().map(norm_case).collect();
428438
if let Some(conda_dir) = get_conda_dir_from_exe(conda_executable) {
429439
known_paths.push(conda_dir);
430440
}
@@ -436,6 +446,13 @@ pub fn get_known_conda_install_locations(
436446
if let Some(mamba_dir) = get_conda_dir_from_exe(&find_mamba_binary(env_vars)) {
437447
known_paths.push(mamba_dir);
438448
}
449+
450+
known_paths = known_paths
451+
.into_iter()
452+
.filter(|path| path.exists())
453+
.map(norm_case)
454+
.map(restore_existing_leaf_case)
455+
.collect();
439456
known_paths.sort();
440457
known_paths.dedup();
441458

‎crates/pet-conda/tests/environment_locations_test.rs‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,3 +205,46 @@ fn skips_path_lookup_when_conda_executable_provided() {
205205
locations
206206
);
207207
}
208+
209+
#[cfg(windows)]
210+
#[test]
211+
fn deduplicates_windows_install_aliases_and_preserves_disk_casing() {
212+
use common::create_env_variables;
213+
use pet_conda::environment_locations::get_conda_environment_paths;
214+
use pet_fs::path::norm_case;
215+
use std::fs;
216+
217+
let temp_dir = tempfile::tempdir().expect("failed to create temporary test directory");
218+
let home = temp_dir.path();
219+
let install = home.join("Miniconda3");
220+
let child = install.join("envs").join("MyEnv");
221+
222+
fs::create_dir_all(install.join("conda-meta"))
223+
.expect("failed to create base conda-meta directory");
224+
fs::create_dir_all(install.join("condabin")).expect("failed to create base condabin directory");
225+
fs::create_dir_all(child.join("conda-meta"))
226+
.expect("failed to create child conda-meta directory");
227+
228+
let conda_state = home.join(".conda");
229+
fs::create_dir_all(&conda_state).expect("failed to create .conda directory");
230+
fs::write(
231+
conda_state.join("environments.txt"),
232+
format!("{}\n{}\n", install.display(), child.display()),
233+
)
234+
.expect("failed to write environments.txt");
235+
236+
let mut env = create_env_variables(home.to_path_buf(), home.to_path_buf());
237+
env.userprofile = Some(home.to_string_lossy().into_owned());
238+
239+
let environments = get_conda_environment_paths(&env, &None);
240+
let normalized_home = norm_case(home);
241+
let mut local_environments = environments
242+
.into_iter()
243+
.filter(|path| path.starts_with(&normalized_home))
244+
.collect::<Vec<_>>();
245+
local_environments.sort();
246+
247+
let mut expected = vec![norm_case(install), norm_case(child)];
248+
expected.sort();
249+
assert_eq!(local_environments, expected);
250+
}

‎crates/pet/tests/e2e_performance.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ static REQUEST_ID: AtomicU32 = AtomicU32::new(1);
2929
/// Number of iterations for statistical tests
3030
const STAT_ITERATIONS: usize = 10;
3131
const PERFORMANCE_METRICS_SCHEMA_VERSION: u8 = 2;
32+
const PERFORMANCE_INVENTORY_SCHEMA_VERSION: u8 = 2;
3233
const STDERR_TAIL_LINES: usize = 100;
3334

3435
/// Statistical metrics with percentile calculations
@@ -1571,6 +1572,7 @@ fn test_performance_summary() {
15711572
// Existing top-level refresh fields remain warm-cache values for schema compatibility.
15721573
let json_output = serde_json::to_string_pretty(&json!({
15731574
"metrics_schema_version": PERFORMANCE_METRICS_SCHEMA_VERSION,
1575+
"inventory_schema_version": PERFORMANCE_INVENTORY_SCHEMA_VERSION,
15741576
"server_startup_ms": startup_stats.p50().unwrap_or(0),
15751577
"full_refresh_ms": warm_refresh_stats.p50().unwrap_or(0),
15761578
"cold_refresh_ms": cold_refresh_stats.p50().unwrap_or(0),

‎docs/QUALITY_SNAPSHOTS.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ PET uses pull-request snapshots to prevent performance and coverage drift. Each
77
The performance workflow runs 10 paired cache-cold/cache-warm JSON-RPC iterations on Linux, Windows, and macOS, plus 10 untimed cache-cold diagnostic iterations. A comparison is valid only when:
88

99
- current and baseline metrics contain at least five samples for every required distribution;
10-
- environment and manager counts match exactly; and
10+
- environment and manager counts match exactly within the same inventory schema; and
1111
- the benchmark command and JSON extraction both succeed.
1212

1313
A metric blocks when it exceeds both its absolute and relative budget:
@@ -32,6 +32,8 @@ The Windows warm full-refresh P50 budget was recalibrated in issue #513 from fiv
3232

3333
Schema v2 records `full_refresh` and `time_to_first_env` from the warm member of each pair and adds cold refresh/time-to-first distributions. During its one-time rollout, comparisons against a schema-v1 base checked cold P50 against explicit absolute ceilings of 500ms on Linux, 750ms on Windows, and 1,000ms on macOS. Schema-v2-to-v2 comparisons use the table's dual budgets.
3434

35+
Inventory schema v2 treats Windows Conda installation paths that differ only by on-disk casing as one logical workload entry. During the one-time v1-to-v2 transition, the report explicitly identifies the schema change and permits the expected count mismatch. Once the v2 baseline is published, exact environment and manager count matching resumes automatically.
36+
3537
The cold P50 budgets were calibrated in issue #509 using two unchanged-head all-platform runs and the final pull-request validation.
3638

3739
The dual budget avoids failing on tiny percentage changes while still blocking material latency regressions. Warm tail metrics remain mandatory; cold P95 remains diagnostic because a single host event can dominate it, while cold P50 blocks delays that affect the independent cold iterations consistently.

‎scripts/quality_snapshot.py‎

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ def regressed(self) -> bool:
106106
),
107107
}
108108
PERFORMANCE_METRICS_SCHEMA_VERSION = 2
109+
PERFORMANCE_INVENTORY_SCHEMA_VERSION = 2
109110
COLD_REFRESH_SPEC = MetricSpec('Cold refresh P50', 'cold_refresh', 'p50')
110111
COLD_DIAGNOSTIC_SPECS = (
111112
MetricSpec('Cold refresh P95', 'cold_refresh', 'p95'),
@@ -196,6 +197,20 @@ def performance_schema_version(snapshot: dict[str, Any], source: str) -> int:
196197
return version
197198

198199

200+
def inventory_schema_version(snapshot: dict[str, Any], source: str) -> int:
201+
version = require_integer(
202+
snapshot.get('inventory_schema_version', 1),
203+
f'{source}.inventory_schema_version',
204+
minimum=1,
205+
)
206+
if version > PERFORMANCE_INVENTORY_SCHEMA_VERSION:
207+
raise SnapshotError(
208+
f'{source}.inventory_schema_version {version} is newer than supported version '
209+
f'{PERFORMANCE_INVENTORY_SCHEMA_VERSION}'
210+
)
211+
return version
212+
213+
199214
def cold_refresh_budget(platform: str) -> RegressionBudget:
200215
key = platform_key(platform)
201216
try:
@@ -230,16 +245,25 @@ def compare_performance(
230245
f'{baseline_version}'
231246
)
232247

248+
current_inventory_version = inventory_schema_version(current, 'current')
249+
baseline_inventory_version = inventory_schema_version(baseline, 'baseline')
250+
if current_inventory_version < baseline_inventory_version:
251+
raise SnapshotError(
252+
f'Current inventory schema {current_inventory_version} is older than baseline '
253+
f'inventory schema {baseline_inventory_version}'
254+
)
255+
233256
current_envs = require_integer(current.get('environments_count'), 'current.environments_count', minimum=1)
234257
baseline_envs = require_integer(baseline.get('environments_count'), 'baseline.environments_count', minimum=1)
235258
current_managers = require_integer(current.get('managers_count'), 'current.managers_count')
236259
baseline_managers = require_integer(baseline.get('managers_count'), 'baseline.managers_count')
237260

238261
failures: list[str] = []
239-
if current_envs != baseline_envs:
240-
failures.append(f'Environment inventory changed: current={current_envs}, baseline={baseline_envs}')
241-
if current_managers != baseline_managers:
242-
failures.append(f'Manager inventory changed: current={current_managers}, baseline={baseline_managers}')
262+
if current_inventory_version == baseline_inventory_version:
263+
if current_envs != baseline_envs:
264+
failures.append(f'Environment inventory changed: current={current_envs}, baseline={baseline_envs}')
265+
if current_managers != baseline_managers:
266+
failures.append(f'Manager inventory changed: current={current_managers}, baseline={baseline_managers}')
243267

244268
comparisons: list[PerformanceComparison] = [
245269
MetricComparison(
@@ -353,6 +377,8 @@ def performance_report(
353377
current: dict[str, Any],
354378
baseline: dict[str, Any],
355379
) -> str:
380+
current_inventory_version = inventory_schema_version(current, 'current')
381+
baseline_inventory_version = inventory_schema_version(baseline, 'baseline')
356382
rows = []
357383
has_legacy_cold_baseline = False
358384
for comparison in comparisons:
@@ -392,12 +418,19 @@ def performance_report(
392418
'',
393419
'> Cold refresh uses a platform absolute ceiling while the exact base has legacy metrics.',
394420
])
421+
if current_inventory_version > baseline_inventory_version:
422+
report.extend([
423+
'',
424+
'### Inventory schema transition',
425+
f'- Inventory schema transitioned from v{baseline_inventory_version} to '
426+
f'v{current_inventory_version}; exact count matching is skipped for this comparison.',
427+
])
395428
if failures:
396429
report.extend(['', '### Blocking findings', *[f'- {failure}' for failure in failures]])
397430
report.extend([
398431
'',
399432
'> A regression must exceed both the documented absolute and relative budget. '
400-
'Environment and manager inventories must match exactly.',
433+
'Environment and manager inventories must match exactly within the same inventory schema.',
401434
])
402435
return '\n'.join(report) + '\n'
403436

‎scripts/tests/test_quality_snapshot.py‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def performance_snapshot(
2727
*, refresh_p50=100, refresh_p95=500, startup_p50=10, startup_p95=20,
2828
first_p50=15, first_p95=30, cold_p50=200, cold_p95=500,
2929
cold_first_p50=25, cold_first_p95=50, environments=5, managers=1,
30-
schema_version=1
30+
schema_version=1, inventory_schema_version=None
3131
):
3232
snapshot = {
3333
'server_startup_ms': startup_p50,
@@ -55,6 +55,8 @@ def performance_snapshot(
5555
'p50': cold_first_p50,
5656
'p95': cold_first_p95,
5757
}
58+
if inventory_schema_version is not None:
59+
snapshot['inventory_schema_version'] = inventory_schema_version
5860
return snapshot
5961

6062

@@ -282,6 +284,44 @@ def test_inventory_mismatch_fails(self):
282284
self.assertTrue(any('Environment inventory changed' in failure for failure in failures))
283285
self.assertTrue(any('Manager inventory changed' in failure for failure in failures))
284286

287+
def test_inventory_schema_transition_allows_count_change(self):
288+
current = performance_snapshot(
289+
environments=6,
290+
managers=1,
291+
inventory_schema_version=2,
292+
)
293+
baseline = performance_snapshot(environments=8, managers=2)
294+
295+
comparisons, failures = compare_performance(current, baseline, 'Windows')
296+
report = performance_report('Windows', comparisons, failures, current, baseline)
297+
298+
self.assertEqual(failures, [])
299+
self.assertIn('Inventory schema transitioned from v1 to v2', report)
300+
301+
def test_same_inventory_schema_still_requires_matching_counts(self):
302+
current = performance_snapshot(environments=6, inventory_schema_version=2)
303+
baseline = performance_snapshot(environments=8, inventory_schema_version=2)
304+
305+
_, failures = compare_performance(current, baseline, 'Windows')
306+
307+
self.assertTrue(any('Environment inventory changed' in failure for failure in failures))
308+
309+
def test_older_current_inventory_schema_is_invalid(self):
310+
with self.assertRaisesRegex(SnapshotError, 'older than baseline inventory schema'):
311+
compare_performance(
312+
performance_snapshot(),
313+
performance_snapshot(inventory_schema_version=2),
314+
'Windows',
315+
)
316+
317+
def test_newer_inventory_schema_is_invalid(self):
318+
with self.assertRaisesRegex(SnapshotError, 'newer than supported version'):
319+
compare_performance(
320+
performance_snapshot(inventory_schema_version=3),
321+
performance_snapshot(),
322+
'Windows',
323+
)
324+
285325
def test_missing_metric_is_invalid(self):
286326
current = performance_snapshot()
287327
del current['stats']['full_refresh']['p95']

0 commit comments

Comments
 (0)