Make --version report install provenance - #85
Conversation
Both builds report the same version, so `dl --version` could not say which one had just run — the name on PATH was the only distinguishing thing, and a `dl` and a `dl-next` of the same version printed identical output. Read the installed dist's PEP 610 `direct_url.json`: `dir_info.editable` marks an editable install and `url` names the tree it resolves to, decoded as the file:// URI it is. No path is stat'd and no install location is pattern-matched, so nothing here assumes where either build lives. The addition is strictly non-fatal. A dist with no direct-url metadata, with metadata that does not parse, or with metadata missing the keys, falls back to the bare version rather than raising — a broken `--version` is worse than an ambiguous one. A non-editable install's output is unchanged. `aid --version` inherits this, since it renders the same version string.
Reviewer's GuideAdd install-provenance reporting to get_version() so editable dev installs are distinguishable from released builds, backed by PEP 610 direct_url.json metadata, with robust degradation, tests, and documentation updates. Sequence diagram for get_version install-provenance reportingsequenceDiagram
actor User
participant dl as dl_cli
participant get_version
participant _install_provenance
participant importlib_metadata as importlib_metadata
participant direct_url_json as direct_url_json
User->>dl: run --version
dl->>get_version: get_version()
get_version->>importlib_metadata: pkg_version(devlaunch)
alt PackageNotFoundError
get_version-->>dl: unknown
else Version found
get_version->>_install_provenance: _install_provenance()
_install_provenance->>importlib_metadata: distribution(devlaunch)
importlib_metadata->>direct_url_json: read_text(direct_url.json)
_install_provenance->>_install_provenance: json.loads(raw)
alt [dir_info.editable is true and url is file]
_install_provenance-->>get_version: dev, editable from <tree>
get_version-->>dl: <version> (dev, editable from <tree>)
else No usable provenance
_install_provenance-->>get_version: None
get_version-->>dl: <version>
end
end
dl-->>User: print version string
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #85 +/- ##
==========================================
+ Coverage 89.24% 89.28% +0.04%
==========================================
Files 12 12
Lines 1636 1661 +25
==========================================
+ Hits 1460 1483 +23
- Misses 176 178 +2
🚀 New features to boost your workflow:
|
blooop
left a comment
There was a problem hiding this comment.
This was generated by AI during review.
Reviewed at head 12fb3101b2ff2094d3bad44cc0a5beabe349067e against the merge-base with main (b304cb6), in a fresh context that did not write the code. Two axes run independently and not merged. Findings were checked by execution in a scratch clone, not by reading — including mutation-testing the implementation against the new tests, and running the new code against the real direct_url.json of both installs on a machine that has them.
Standards
Ruff, pylint and ty pass; broad-exception-caught is disabled repo-wide (pyproject.toml:115) with precedent in devlaunch/worktree/branch_manager.py:117 and devlaunch/dl.py:1111, so the bare except Exception is not a finding on its own. CLAUDE.md is a symlink to AGENTS.md, so the one edit covers both. No /home/<user> path anywhere in the diff — the tests use /srv/checkouts. Docstring density matches repo precedent.
Nits
- Four guards are unreachable and produce nothing the blanket handler doesn't.
devlaunch/dl.py:89(isinstance(info, dict)),:93(isinstance(url, str)),:96(scheme != "file"),:99(if not tree). Each returnsNone— exactly whatexcept Exceptionat:102already yields for the same input. Proven by mutation: deleting:93-94,:96-97or:99-100individually leaves all 12 version tests green. Coverage confirms:97and:100are never executed by any test. There is repo precedent for isinstance-checking parsed JSON (devlaunch/worktree/storage.py:104,143,153), but it does not transfer — there the check sits under a narrowexcept (OSError, ValueError)and produces a distinct action. Speculative Generality;:86and thedir_info.get("editable")check at:90are the two that describe the real PEP 610 schema. - The format string is assembled in two places.
devlaunch/dl.py:101returns the fragment"dev, editable from {tree}";:113supplies the parens. Neither location owns the rendered string, and the fragment is only well-formed inside the parens its caller adds. Returning the tree path and lettingget_versionwritef"{base} (dev, editable from {tree})"puts the format in one place. aid --helpwas left behind.devlaunch/aid.py:169still reads--version Show versionwhiledl's help line was elaborated atdevlaunch/dl.py:1171, andREADME.md:237asserts "aid --versionreports the same thing under its own name." GivenCLAUDE.md's docs-in-sync rule, this is the one line now out of step with behavior the PR itself changed.- Placement.
devlaunch/dl.pyis 1569 lines and already owns orchestration, cache, help and dispatch. Version+provenance is now ~45 self-contained lines with its own dependencies (importlib.metadata,urllib) and a cross-module consumer atdevlaunch/aid.py:194. Adevlaunch/version.pywould match the layout ofcompletion.py/gh_auth.py/workspace_id.pyand stopaidimporting the 1569-line module for one string. Not obligatory — but this PR is the moment "version" stopped being one line. 0.0.9is hardcoded in both worked examples (README.md:231,AGENTS.md:42). They go stale at the next bump;dl <version> (dev, editable from …)reads the same and cannot drift. Related:README.md:225-239andAGENTS.md:41-44are now near-duplicate prose that must move together.- The dist name
"devlaunch"is a literal in two places —devlaunch/dl.py:85and:109. Minor, but they must agree or the provenance describes a different dist than the version.
Spec
Spec is #84. Every "Done when" bullet is met.
| Spec line | Result |
|---|---|
"A test asserts an editable install (dir_info.editable == true) yields output naming it as dev and containing the tree path from url" |
Met — test/test_dl.py:817-828 |
"A test asserts a non-editable install yields the current bare dl <version> output, unchanged" |
Met — test/test_dl.py:838-843 |
| "Tests cover the three degradation cases above, each falling back to bare output rather than raising" | Met, plus a fourth (reader raises) — test/test_dl.py:845-875 |
"pixi run ci is green" |
All GitHub checks green on head; 313 tests pass in the scratch checkout |
"README updated if it shows --version output" |
Met — README.md:223-238; AGENTS.md:41-44 corrected, and it previously asserted the opposite |
Each hard constraint verified independently, by execution:
- "Provenance detection is strictly additive: it must never make
--versionfail." Held. The probe you were most likely to have missed —distribution()itself raising — is covered:PackageNotFoundErrorsubclassesImportErrorsubclassesException, sodevlaunch/dl.py:102catches it. Executed and confirmed bare for:PackageNotFoundErrorfromdistribution(),OSErrorfrom the reader,MemoryError,None,"","{not json",[1,2,3],null,dir_infoa string,urla dict,editable: 0.KeyboardInterruptandSystemExitcorrectly still propagate.jsonis imported atdevlaunch/dl.py:23, so there is no NameError path. - "a non-editable install yields the current bare output, unchanged." Held — and verified against reality, not just against a stub. The released conda build on this machine records
{"dir_info": {}, "url": "file:///home/runner/work/devlaunch/devlaunch/output/bld/rattler-build_devlaunch_.../work"}(the recipe'spip install .atconda.recipe/recipe.yaml:19does write PEP 610 metadata — it is present but non-editable, not absent). Fed through the new code it returns exactly0.0.9. This closes the PR's own "not verified" caveat on the released path. - "do not stat paths or pattern-match against
~/.pixior~/.local, and do not hardcode any/home/<user>path." Held.devlaunch/dl.py:69-103contains nostat/exists/is_dir/Path(, no.pixi/.local, no/home/. - "
urlsupplies the tree to name", parsed not stripped. Held — and the test discriminates: swappingurl2pathname(parsed.path)forurl.replace("file://","")failstest/test_dl.py:830-835. Executed:%20→space,%C3%A5→å,%23→#. - "
PackageNotFoundErroris already handled at the existing call site; keep that behavior." Held —devlaunch/dl.py:110-112short-circuits to"unknown"before provenance runs.
No pre-existing test was weakened. git diff b304cb6...HEAD -- test/ touches only test/test_dl.py and contains zero deletion lines. TestGetVersion (test/test_dl.py:782-798) is untouched, as is test/unit/test_aid.py — including test_version_matches_dl at :162, which still compares CLI output to a live get_version().
Nothing else parses --version output. Grepped scripts/, conda.recipe/, .github/, completions, pyproject.toml. Consumers are devlaunch/aid.py:194, dev.sh:108, dev-pixi.sh:20 — all print for humans. devlaunch/completions/dl.bash knows the flag name only.
Mutation testing — the tests are not vacuous. Two deliberate breaks in a scratch copy each turn a test red:
| Mutation | Caught by |
|---|---|
drop the dir_info.get("editable") guard |
test_non_editable_install_reports_bare_version (test/test_dl.py:843) |
url2pathname(parsed.path) → url.replace("file://","") |
test_percent_encoded_tree_path_is_decoded (test/test_dl.py:835) |
All 7 new tests also fail against the base devlaunch/dl.py.
Nits
- Red-before-green is not evidenced by the history.
git log b304cb6..HEADis two commits:fda3cb3carries implementation and all 7 tests together, then12fb310is a codespell fixup. The tests are real and do go red against the base — but they fail there withAttributeError: module 'devlaunch.dl' does not have the attribute 'distribution', i.e. because the@patchtarget is missing, not because they observe the old behavior. That is a mechanically-red step, not a behaviorally-red one. The four degradation tests are additionally satisfied by any implementation that never adds provenance, which is inherent to negative tests, but worth naming. dir_info.get("editable")is a truthiness test (devlaunch/dl.py:90). A JSON"editable": "false"— a string — reports the install as dev. PEP 610 mandates a boolean and no real installer emits a string, so the risk is ~0;dir_info.get("editable") is Truewould be exact.parsed.netlocis discarded (devlaunch/dl.py:95-98).file://otherhost/srv/treenames/srv/treeas a local tree. Unreachable from a compliant installer, which writes an empty authority.- A raw unencoded
#or?in the url truncates silently rather than falling back —file:///srv/a#b/devlaunch→dev, editable from /srv/a, a wrong path. Not a spec breach (--versiondoes not fail), and I confirmedpathname2urlencodes both, so/srv/a#b/devlaunchis actually written asfile://///srv/a%23b/devlaunchand decodes correctly. Unreachable in practice. - Provenance describes the installed dist's metadata, not the module that actually got imported. Running
python -m devlaunch.dlfrom a checkout inside an environment holding a released devlaunch prints the bare version while executing tree code — the exact ambiguity the ticket set out to remove. Closing it would mean comparingdevlaunch.__file__to the reported tree, which the ticket forbids ("do not stat paths"), so this is a consequence of the constraint rather than a defect. A line in the_install_provenancedocstring naming the limit would be worth having.
Verdict
Approve (with nits). No blocking findings on either axis.
The mechanism is the right one: the fact comes from metadata the installer wrote, so it holds for any user and any prefix, and the CLAUDE.md path rule is respected without contortion. Every degradation path I could construct — including the one the review brief flagged as the likely miss, distribution() raising PackageNotFoundError — returns the bare version, and KeyboardInterrupt/SystemExit correctly still propagate. The released path, which the PR body listed as unverified, checks out against the real conda build's actual direct_url.json.
Nothing here needs to block the merge. The two I would actually spend a commit on:
- Nit 3 —
devlaunch/aid.py:169help line, since it is a docs-sync gap this PR created andCLAUDE.mdmakes that rule explicit. - Nit 1 — dropping the three unreachable guards at
devlaunch/dl.py:93,96,99, which no test pins and the blanket handler already covers.
Both are one-line changes and neither affects behavior.
PR #85 merged into main after this release branch was cut, so the 0.0.10 section did not mention it. publish.yml parses these notes for the release, so an omission here is an omission from the published release notes.
PR #85 merged into main at 22:31Z; the 0.0.10 release branch had been cut at 22:15Z and merged without picking it up, so the shipped notes omit a change that is in the shipped code. Documentation only — no version bump, so publish.yml sees no version increase and will not republish.
The problem
dlon PATH is the released pixi-global build;dl-nextis an editable install of a working tree. Both printdl 0.0.9— the version came fromimportlib.metadata.version, which is identical for the two. So the only thing distinguishing a released run from a half-finished working tree was the name typed to invoke it, and any transcript, bug report, or CI log recording just--versionoutput was ambiguous about which build produced it.The change
get_version()now appends an install-provenance clause when the installed dist reports itself as editable:Provenance is read from the dist's own PEP 610
direct_url.jsonviaimportlib.metadata.dir_info.editable == trueis the discriminator andurlsupplies the tree to name. Theurlis afile://URI and is parsed as one (urlparse+url2pathname), not string-stripped, so a percent-encoded path decodes correctly.Nothing here stats a path, pattern-matches an install location, or hardcodes a
/home/<user>path — the fact comes from metadata the installer wrote, so it holds for any user and any install prefix.aid --versioninherits this with no change of its own, since it renders the same version string under its own name.Non-fatal by construction
A broken
--versionis worse than an ambiguous one, so provenance detection can only ever add to the output, never prevent it. Every failure mode falls back to today's bare<version>:direct_url.jsonat all (plain wheel/sdist install)direct_url.jsonpresent but not parseable as JSONurl, ordir_infonot a mappingPackageNotFoundErrorfrom the version lookup — the pre-existing"unknown"handling at that call site is untouchedScope
Version string only. No change to any other command, to how versions are assigned, or to packaging.
Docs kept in sync per CLAUDE.md: the README's global-commands table and a short worked example of both outputs, and the
dl --helpline for--version. AGENTS.md/CLAUDE.md carried a now-false claim thatdl-next --version"reports the package version, not its provenance" — corrected.Tests
Seven tests in
TestVersionProvenance, all drivingget_version()with the installed-dist metadata reader stubbed at that system boundary (no devlaunch function of ours is mocked):url→ bare versionThe pre-existing
PackageNotFoundErrortest and the CLI-level--versiontests still pass unmodified.Verified / not verified
Verified against real metadata on a machine that has both installs: the editable path was confirmed end to end by actually running
--version(andaid --version) out of an editable install and seeing it name the right tree; the released path was confirmed by feeding the released build's realdirect_url.jsonbytes through the new code and getting a bare0.0.9.Not verified: a released build carrying this code has not been built and run — the released side was exercised through its real metadata, not through a real conda-channel install. The degradation cases are covered by tests only; no install genuinely lacking
direct_url.jsonwas constructed to confirm in the wild.pixi run cigreen locally: 815 passed, 8 deselected; ruff format 40 files unchanged; ruff check all passed; pylint 10.00/10; ty all checks passed; coverage 89% total,devlaunch/dl.py90%.Closes #84
Summary by Sourcery
Make the CLI version output include install provenance for editable dev installs while preserving existing behavior for released builds.
Enhancements:
Documentation:
Tests: