Repository navigation
Three correct-but-separate spellings of the duplicate-qn marker grammar remain after #2017 #2114
Description
Follow-up from #2017 / PR #2113, which routed ten readers through a new shared helper (codebase_rag/utils/qn_markers.py).
Three sites were already correct before that PR and were left alone, because changing working code was out of scope. They are not bugs today, but each is an independent spelling of one grammar:
| site | spelling |
|---|---|
duplicates.py:251 _DUP_QN_MARKER_RE |
unanchored regex @\d+(?:_\d+)?, used with .sub("") |
trace/resolution.py:83 _natural_qualified_name |
rpartition + suffix.replace(COLUMN_MARKER, "").isdigit() |
class_ingest/mixin.py:1303 (partial key) |
rpartition + tail[:1].isdigit() |
Verified during the #2113 review: all three agree with the shared helper on @event, @event@12, @event@12_5 and pkg.M@12_5, so nothing is broken now.
Why it is still worth closing
The defect #2017 fixed was not that any one reader was wrong — it was that eight readers each spelled the grammar themselves, so a change to the producer could invalidate some and not others. Three surviving spellings preserve exactly that property in miniature.
The producer is function_registry.register_unique_qn, which appends @<line> then optionally _<col>. If that ever changes, qn_markers._MARKER_RE is built from the producer's own constants and follows automatically; these three do not. Two of them hard-code the digit test, and mixin.py's tail[:1].isdigit() is sufficient only because rpartition already took the last @ — a correctness argument that lives in the reader rather than in the grammar.
Suggested change
Route all three through qn_markers. Each needs a slightly different entry point, so this is not a pure find-and-replace:
duplicates._qn_normalizedstrips the marker from anywhere in a dotted qn before a hierarchy comparison, not just the end, so it needs the unanchored form. Either export the pattern fromqn_markersor add astrip_all_markers.trace/resolutionandmixinboth want end-anchored semantics and can callstrip_dup_markerdirectly.
Worth a test that all four spellings agree over a shared table of inputs, so a future divergence fails loudly rather than silently.
Not urgent
No user-visible defect. Filing it so the consolidation #2017 started is finished rather than left three-quarters done — the exact shape that let #2014 fix one reader and leave seven.
Activity
vitali87 commentedon Sep 21, 2026
A twelfth site, and a correction to this issue as filed: consolidation here is not a substitution, and I would have got that wrong.
PR #2011 (
eee07c42, still open) fixes the same defect class incodebase_rag/parsers/csharp/type_inference.py::_dotted_type_path_qn— a written type path was matched by splitting at the FIRST@, soproj.src.Lib.Lib.@event@12truncated toproj.src.Lib.Lib.and the real type was rejected. That file is not onmainyet, so it is not in PR #2113's sweep. The fix is correct; it reaches for_DUP_QN_MARKER_REfromduplicates.py, which is one of the three spellings this issue tracks.Why that call site cannot take
qn_markers.strip_dup_marker. It needs the UNANCHORED form, and the difference is load-bearing:duplicatesunanchored.sub("")qn_markers.strip_dup_marker(end-anchored)proj.Lib.Helper@12proj.Lib.Helperproj.Lib.Helperproj.Lib.@event@12proj.Lib.@eventproj.Lib.@eventproj.Lib@7.Helperproj.Lib.Helperproj.Lib@7.Helperproj.Lib@7.Helper@12proj.Lib.Helperproj.Lib@7.HelperA marker can sit on an INNER segment — a duplicate-suffixed outer class with a nested type (
Lib@7.Helper)._dotted_type_path_qnmatches a candidate by its whole path, so it must strip markers from every segment; an end-anchored strip would leave the inner one and the match would silently fail.So the shape of the consolidation is:
qn_markersshould expose BOTH forms with the distinction documented (end-anchored for a leaf or a whole-qn tail, unanchored for a path match across segments), and each of the sites listed above adopts whichever it actually needs. Reaching for one function everywhere would introduce a bug in exactly the site that motivated this comment.That makes the issue more valuable rather than less: the reason there are four spellings is that two different semantics are in play and nothing names the difference, so each author re-derives which one they need. Naming them is most of the fix.
Also raises the count: eight readers fixed in #2113, two in
evals/, three pre-existing correct ones listed above, plus this one. Worth doing the consolidation after #2011 and #2113 both land, not before — they touch overlapping ground right now.vitali87 commentedon Sep 21, 2026
The full four-way disagreement table, generated rather than reasoned, so this issue carries evidence instead of my summary of it.
All four spellings, over the shapes that matter:
duplicates)qn_markers)trace/resolutionmixin:1303Box@12BoxBoxBoxBoxBox@12_5BoxBoxBoxBox@event@event@event@event@event@event@12@event@event@event@event@event@12_4@event@event@event@event@lock@lock@lock@lock@lockproj.Lib@7.Helperproj.Lib.Helperproj.Lib@7.Helperproj.Lib@7.Helperproj.Libproj.Lib@7.Helper@12proj.Lib.Helperproj.Lib@7.Helperproj.Lib@7.Helperproj.Lib@7.Helperproj.@lock.Inner@1@1plainplainplainplainplainThree rows disagree, and each names a real distinction rather than a bug:
proj.Lib@7.Helper— inner-segment markers. Only the unanchored form strips them. A site matching a candidate by its WHOLE path needs that; a site handed a leaf must not have it. This is the axis I described earlier.proj.Lib@7.Helperundermixin:1303->proj.Lib. Not a defect: that code runs on a SUFFIX relative to a module qn, never on a full dotted path, so the input cannot occur there. It is a precondition that lives in the caller and nowhere in the function — exactly the kind of thing that breaks when code is moved.@1—trace/resolutionkeeps it, the others strip it.tracerequires the whole suffix to be digits after removing the column marker; the others test only the first character.@1is not a legal identifier in any language here, so both answers are defensible and neither is reachable today.So the consolidation is: two exported functions (
strip_dup_markerend-anchored, and an unanchored variant), each site picking by whether it holds a leaf or a path, plus a shared table like this one as a test so a fourth spelling cannot drift in silently. Row 2 also argues for asserting the precondition where it is relied on.Reproduce with
uv run pythonoverduplicates._DUP_QN_MARKER_RE,qn_markers.strip_dup_marker,trace.resolution._natural_qualified_name, and the inlinerpartitionatmixin.py:1303.Cross-checked with the peer session working #2011: the three sites they measured as broken (
dead_code,graph_updater,method_override) are broken onmainand already fixed in #2113, so they do not belong here. This issue stays scoped to the three correct-but-divergent spellings.refactor: every duplicate-marker reader goes through qn_markers (vita…