Reviewed PR 337 (alex.leake/feat/s3-optimisations) against merge base 790fac112990338295efc7611342b2aa69fed89e and confirmed that the PR description calls the unknown-size range probe a fallback for older API versions, but gives no rollout requirement or compatibility window.
Added shared Pydantic storage response contracts used by both vxdata-api and vxdata-sdk (27935a05).
Removed superseded download and list helpers, moving TLS coverage onto the active transfer engine (77ec7920).
Made TransferConfig authoritative for concurrency. Retained max_workers as an explicit deprecated compatibility shim and reject calls that pass both knobs (a0aa2bff).
Added a reusable StorageTransport that resolves API versus Direct-S3 mode once and reuses one MinIO client. Added a three-line Direct-S3 README example (6a36771d).
Moved JSONL telemetry onto an isolated non-propagating logger, preserved application logger policy, and made configured path errors visible (cac69225).
After an explicit product decision to prefer simplification over backwards compatibility, removed the dynamic size-probe scheduler, old list envelope, omitted-size response acceptance, and max_workers shim (6e05cad0). The commit removed 302 lines and added 164; exact range and byte-count validation replaced the probe-specific response branches.
Bumped vxdata-schemas, vxdata-sdk, and vxdata-api from 2.1.0 to the breaking release 3.0.0 (94f8a335); all Pixi lock checks remained current and installed metadata reported 3.0.0.
Updated infra_k8s from latest main, pinned both staging and production API deployments to vxdata-api:3.0.0, and opened empty-description deployment PR virdx/infra_k8s#182 from commit 7d0a2b0.
Retained SDK-managed JSONL telemetry but removed its dedicated 51-line test file and trimmed the formatter/handler setup (9da8e6c6, net -54 lines).
Cached the S3 endpoint override alongside Direct-S3 mode for each StorageTransport (99c5ffae, net -1 line). Direct-S3 usage remains documented in packages/vxdata-sdk/README.md through environment variables; existing client.storage calls require no alternate Python API.
Compiled the full user-facing storage call graph and recorded deferred batching/simplification work in virdx/mono#347, explicitly outside PR #337.
Pushed the nine maintainability commits through 99c5ffae to PR #337 and surgically corrected only its stale compatibility, max_workers, Version 3 contract, and validation claims. The published follow-up delta is 405 additions and 460 deletions (net -55).
Final validation passed: schemas lint/typecheck/5 tests; SDK lint/typecheck/38 tests; API lint/typecheck/5 focused storage tests; ast-grep and diff checks.
Lessons Learned: Pitfalls
The PR contains more than 2,000 added lines, but its description only labels the old-API probe as transitional. It does not explain who needs mixed-version compatibility or when the path can be removed. The path was removed only after the user explicitly chose simplification over mixed-version compatibility.
The monorepo boundary command could not run in this worktree because root Nx modules were not installed. It also caused bunx to resolve packages before failing, so targeted Pixi checks were the useful validation path for this Python-only change.
max_workers existed at the merge base, so it was initially retained pending an explicit compatibility decision. It was removed only after the user chose the breaking simplification.
The infra repository manifest lint scripts require yq, which was not installed. The two changed multi-document YAML manifests were parsed with PyYAML and passed git diff --check; the unrelated untracked .DS_Store was left untouched.
Lessons Learned: Improvements
Storage compatibility changes should document the supported API/SDK version matrix, rollout order, and removal condition in the PR description or a linked issue.
Reviews should compare public method signatures to the merge base before recommending deletion of duplicate-looking options.
Python worktree setup guidance should state whether root bun install is required for boundary validation or provide a boundary check that does not dynamically resolve missing Nx packages.
