Skip to content

fix(tui): the package lookup compares untranslated versions on both sides, so a non-semver ecosystem is misread #148

Description

@justin13888

dependable-tui's package lookup evaluates freshness with check_version on
untranslated strings on both sides of the comparison: the current version it
was given, and the whole list of versions the registry published. The CLI path
translates both before comparing and translates the answer back afterwards. For
Python, C#, and the JVM the two paths therefore answer different questions about
the same package.

Found while reviewing #120 (feat/107-exact-pin-version), which is where the
consequence surfaced. It is pre-existing and separate from that PR's own
defect, which was in exact_pin's guard; #120 fixes the guard and does not
touch this.

Mechanism

crates/dependable-tui/src/data.rs, in lookup:

match checker.fetch_versions(ecosystem, name).await {
    Ok(versions) => {
        let evaluation = check_version("*", &versions, Some(version));

versions is whatever the registry served, in its own dialect, and version is
the Node::version off the selected row. check_version
(crates/dependable-core/src/semver/checker.rs) does:

let mut parsed: Vec<Version> = versions.iter().filter_map(|v| Version::parse(v).ok()).collect();
...
let locked = locked_at.and_then(|s| Version::parse(s).ok());

so any string in either position that is not strict semver is silently
dropped
, not reported.

Compare evaluate_item in crates/dependable-fetch/src/check.rs, which for the
same inputs runs to_semver_versions over the list (per-ecosystem:
pep440_to_semver, nuget_to_semver, maven_to_semver), runs
to_semver_constraint over the constraint, compares in semver, and then maps the
reported versions back to their registry-native spelling through native_for /
in_native_versions — with a comment explaining precisely why the round-trip
matters ("several natives can translate to one semver").

Consequences, both directions

Why it is worth filing rather than fixing in passing

The machinery the TUI needs is private to dependable-fetch:
to_semver_versions, in_native_versions, and native_for are all fn, not
pub fn, inside check.rs. Closing this is therefore a choice between two real
options, and the choice is the substance of the issue:

  1. Publish it. Expose a translating evaluation entry point from
    dependable-fetch (something like evaluate_versions(ecosystem, &versions, current)) and have both the CLI and the TUI call it. Correct by construction,
    but it is a public-API commitment for a crate whose surface is the documented
    integration point for external consumers.
  2. Duplicate it. Reimplement the translate/compare/map-back sequence in
    data.rs. No API commitment, and two copies of a lossy translation that
    already needed a long comment to explain why the mapping back is by parsed
    equality rather than by text — they will drift.

Whichever is chosen, it should be one call, not a patch to the current-version
side alone: both sides of the comparison are affected, and repairing one leaves
the other.

Acceptance

  • A TUI-level test over a JVM (or NuGet, or PEP 440) package whose published list
    contains versions that are not strict semver asserts the pane reports the
    registry's real newest release.
  • A TUI-level test asserts a current version in the ecosystem's own dialect is
    compared as that ecosystem reads it, rather than dropped.
  • The decision between publishing and duplicating is recorded in the PR that
    closes this.

Related: #96 (the same false ok from a version never read), #113 (the CLI-side
NuGet reading), #120 (where this was found).

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions