Skip to content

Fix two classes of wrong answer in Path.relative - #12

Merged
hellerve merged 1 commit into
masterfrom
claude/relative-dot-and-updir
Aug 21, 2026
Merged

Fix two classes of wrong answer in Path.relative#12
hellerve merged 1 commit into
masterfrom
claude/relative-dot-and-updir

Conversation

@carpentry-agent

Copy link
Copy Markdown

Path.relative is a pure lexical function whose whole job is to give an exact
answer, and it returned confidently wrong paths for two ordinary shapes.

1. . was counted as a real path segment

relative filtered only empty components out of the split paths, but
normalize returns the literal string "." for an empty or fully-cancelling
relative path — so that "." survived split and was counted as a segment:

call master correct
(Path.relative "a" ".") (Just @"../a") (Just @"a")
(Path.relative "." "a") (Just @"../.") (Just @"..")
(Path.relative ".." ".") (Just @"../..") (Just @"..")

The first of those is the plainest question there is — where is a, relative
to the current directory
— and it answered ../a.

The fix filters "." alongside "". That is safe because normalize emits
"." only as that whole-path sentinel; it never leaves one inside a path, so
the filter can never drop a meaningful component.

2. A base that stays above the target was answered anyway

If the base still starts with .. once the shared prefix is gone, the two paths
cannot be related lexically at all: from ../b you cannot name a without
knowing the current directory's own name. Master emitted a wrong path regardless:

call master correct
(Path.relative "a" "../b") (Just @"../../a") (Nothing)
(Path.relative "a" "..") (Just @"../a") (Nothing)
(Path.relative "b" "../a") (Just @"../../b") (Nothing)
(Path.relative "../a" "../../b") (Just @"../../a") (Nothing)

Go's filepath.Rel returns an error for exactly this shape ("can't make a
relative to ../b"
). relative already returns (Maybe String) and already
answers Nothing for the other un-relatable pairs — one path absolute and the
other relative, different Windows drive roots — so Nothing is the consistent
answer here and needs no API change.

Because normalize keeps .. only at the front of a relative path, testing the
single component at the split point catches every case; no scan is needed.

What is deliberately unchanged

  • The symmetric target-side case is resolvable and still works:
    (Path.relative "../a" "b")(Just @"../../a"), and
    (Path.relative "../a" "../b")(Just @"../a"). Both are pinned by new
    tests that pass on master too, as regression guards.
  • Absolute paths are untouched, since normalize never emits . or .. for
    them.

Verification

Beyond the seven new unit tests, I ran a differential sweep over all 15625
target/base pairs
built from {a, b, .., .} up to length 3. For every Just
answer it checks the round-trip normalize(base + answer) == normalize(target)
under two disjoint absolute anchors, and for every Nothing it checks the
answer really is anchor-dependent (i.e. no lexical answer exists).

master: pairs=15625 just=15625 | wrong=6624 overeager=0
this:   pairs=15625 just=12401 | wrong=0    overeager=0

Master never answers Nothing for a relative pair and is wrong on 6624 of them;
this branch is wrong on none, and none of its 3224 Nothings is over-eager. The
absolute branch round-trips on every pair both before and after.

Five of the seven new tests fail on origin/master (121 passed / 5 failed with
the new test file against master's path.carp); the other two are the
regression guards above. carp -x test/path.carp (126 passed / 0 failed),
angler, carp-fmt --check and carp -x gendocs.carp are all green.

Notes

  • The relative doc string and the README section gained the third Nothing
    case and a short example block.
  • docs/index.html is regenerated alongside docs/Path.html to keep the two
    byte-identical, which is how master has them. Happy to drop that hunk if you
    would rather sync it at release time.
  • No CHANGELOG entry: this repo has never carried one, so I did not want to
    introduce the file in a bug-fix PR. Say the word and I will add it.

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

`relative` filtered only empty components out of the split paths, but
`normalize` returns the literal "." for an empty or fully-cancelling
relative path, so that "." survived `split` and was counted as a real
segment: (relative "a" ".") answered "../a" instead of "a", and
(relative "." "a") answered "../." instead of "..". Filtering "."
alongside "" fixes it, and is safe because normalize emits "." only as
that whole-path sentinel — it never leaves one inside a path.

A base that still starts with ".." after the shared prefix is stripped
cannot be related to the target lexically at all: from "../b" you cannot
name "a" without knowing the current directory's own name. The old code
emitted a confidently wrong answer ("../../a"). Go's filepath.Rel
returns an error for exactly this shape; `relative` already returns
(Maybe String) and already answers Nothing for the other un-relatable
pairs, so Nothing is the consistent answer and needs no API change.
Because normalize keeps ".." only at the front of a relative path,
testing the single component at the split point catches every case.

The symmetric target-side case is unaffected: (relative "../a" "b") is
resolvable and still answers "../../a". Absolute paths are unaffected
too, since normalize never emits "." or ".." for them.

Verified with a differential sweep over all 15625 target/base pairs
built from {a, b, .., .} up to length 3, checking that every Just
round-trips (normalize(base + answer) == normalize(target)) under two
disjoint absolute anchors, and that every Nothing is genuinely
anchor-dependent. Master fails 6624 of those pairs; this passes all.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build & Tests

carp -x test/path.carp on the branch → 126 passed, 0 failed. CI is green on both runners; this repo's workflow also gates angler, carp-fmt --check and carp -x gendocs.carp.

Findings

The new tests have teeth. I put origin/master's path.carp back underneath the branch's test file and re-ran: 121 passed, 5 failed — exactly the five the description names, and no others:

Test 'relative descends from a . base' failed
Test 'relative walks up to a . target' failed
Test 'relative walks up from a . base' failed
Test 'relative returns Nothing for a base above the current directory' failed
Test 'relative returns Nothing for a base whose residual starts with ..' failed

The two regression guards (../a vs b, and ../a vs ../b) do pass on master, as claimed.

Independent differential — 28224 pairs, zero disagreement. I did not reuse the PR's harness. I built 168 paths (84 relative sequences over {a, b, .., .} of length 1–3, plus the same 84 rooted at /), swept all 168×168 pairs on both master and the branch, and scored every answer against CPython's posixpath.relpath. Answerability is decided by running the oracle under three structurally different anchors (/aa/bb/cc/dd, /zz/yy/xx/ww/vv, /q1/q2/q3/q4/q5/q6) and requiring all three to agree — where they disagree the result depends on the current directory's own name, so Nothing is the only correct answer:

checked/abs/abs 7056   checked/abs/rel 7056   checked/rel/abs 7056   checked/rel/rel 7056

pr-ok/abs/abs   7056   pr-ok/abs/rel   7056   pr-ok/rel/abs   7056   pr-ok/rel/rel   7056
PR disagreements: 0

master-ok/rel/rel                    4014
master-answered-unanswerable/rel/rel 1755
master-wrong-value/rel/rel           1287

The branch agrees with the oracle on every pair, with zero over-eager Nothings. Master is wrong on 3042, and all 3042 are relative/relative — which is an empirical version of the "absolute paths are untouched" claim rather than an argument for it.

Why the one-component test is enough. normalize only appends .. to out when out is empty or its last element is already .. (path.carp:127-134), so in a normalized relative path .. can only occur as a leading run — testing the single component at the split point really does catch every case, no scan needed. It also drops . from out unconditionally and emits "." only as the whole-path sentinel when joined comes out empty, so filtering "." in relative cannot swallow a meaningful component. Both of those are load-bearing for this diff and both check out.

Docs are in sync. carp -x gendocs.carp on the branch leaves the working tree clean, and docs/index.html is still byte-identical to docs/Path.html, which is how this repo keeps the pair.

No dependent is affected. Path.relative has no callers anywhere else in carpentry-org, so the new Nothing cannot break another package.

One item for you rather than for the author: (relative "a" "../b") going from Just "../../a" to Nothing is a semantics change, not only a bug fix, and it is the single judgement call in this PR. I think it is the right one — the old answer is wrong under every anchor, relative already returns Nothing for the other un-relatable shapes, and Go's filepath.Rel errors on exactly this case — and the doc string and README now both say so. But it is the part worth your eye rather than mine.

out/main.c is not regenerated here. It was already four path.carp commits stale on master (last touched by 846c9c6), so that is not this PR's doing.

Verdict: merge

Two real wrong answers, a fix whose safety rests on two properties of normalize that I verified rather than took on trust, and agreement with an independent oracle on all 28224 pairs.

@hellerve
hellerve merged commit e282213 into master Aug 21, 2026
2 checks passed
@hellerve
hellerve deleted the claude/relative-dot-and-updir branch August 21, 2026 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant