Skip to content

fix(content-lane): canonicalize issue-body path tokens in checkContentLaneDeliverable - #9889

Closed
RealDiligent wants to merge 1 commit into
JSONbored:mainfrom
RealDiligent:fix/critical-issue-content-lane-canonicalize-token-9667
Closed

fix(content-lane): canonicalize issue-body path tokens in checkContentLaneDeliverable#9889
RealDiligent wants to merge 1 commit into
JSONbored:mainfrom
RealDiligent:fix/critical-issue-content-lane-canonicalize-token-9667

Conversation

@RealDiligent

Copy link
Copy Markdown
Contributor

Summary

checkContentLaneDeliverable (src/review/content-lane/registry-logic.ts) is the deterministic "did this PR actually deliver the content file its linked issue names?" guard against a test-only PR closing a content issue via a bare Closes #N. It matched changedFiles canonicalized (matchesSpec(canonicalize(file))) but tested the issue-body path tokens raw:

const mentionedPath = extractPathTokens(issueText).find(matchesSpec);   // RAW token

entryFilePattern/providerFilePattern are compiled by globToRegExp, which canonicalizes the glob first and emits ^…$ with no i flag — so a compiled pattern only ever matches lowercase input. An issue body naming its target with any capitalization (Registry/Subnets/Foo.json) therefore yields no mentionedPath, and with no issueTitleImpliesEntryPattern match the check returns not-applicable — silently skipping the deliverable gate, the exact gap the function exists to close. classifyRegistryPrScope in the same file already documents and handles this for changed files; the issue-body side never got the treatment.

Fix

const mentionedPath = extractPathTokens(issueText).find((token) => matchesSpec(canonicalize(token)));

Each token is tested canonicalized, mirroring classifyRegistryPrScope's matchesPattern helper, while .find still returns the original token — so the mentionedPath rendered into the public PR comment quotes the issue body verbatim. An already-lowercase token is byte-identical to today. No change to extractPathTokens, globToRegExp, canonicalize, the compiled-glob i flag, or the issueTitleImpliesEntryPattern fallback.

Tests

Added to test/unit/content-lane-registry-logic.test.ts:

  • A mixed-case body (Registry/Subnets/Foo.json) with no matching changed file ⇒ { verdict: "missing", mentionedPath: "Registry/Subnets/Foo.json" } (original token preserved).
  • The same mixed-case body ⇒ delivered when the PR changes registry/subnets/foo.json.
  • All-lowercase behaviour (both delivered and missing) unchanged.
  • A body path token matching no spec pattern still returns not-applicable (no over-broadening).

The two mixed-case cases fail against the current raw-token query. Full file passes (175/175).

Closes #9667

@RealDiligent
RealDiligent requested a review from JSONbored as a code owner July 29, 2026 19:08
@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - reject/close recommended

Review updated: 2026-07-29 19:19:28 UTC

2 files · 1 AI reviewer · 1 blocker · CI green · clean

🛑 Suggested Action - Reject/Close

Review summary
This is a narrow, well-targeted one-line fix: `checkContentLaneDeliverable` now tests each issue-body path token canonicalized (matching how `classifyRegistryPrScope` already canonicalizes changed files) while `.find` still returns the original token so the public-facing `mentionedPath` still quotes the issue body verbatim. The trace is verifiable in the full file: `globToRegExp`-compiled patterns are canonicalized with no `i` flag, so an uppercase/`./`-prefixed/`\`-separated body token would previously never match, exactly mirroring the documented asymmetry that `classifyRegistryPrScope`'s own comment already calls out for changed files. Tests cover the mixed-case-missing, mixed-case-delivered, all-lowercase-unchanged, and not-over-broadened cases, and CI passed.

Nits — 2 non-blocking
  • The PR title says 'canonicalize issue-body path tokens' but the linked issue orb(content-lane): canonicalize issue-body path tokens in `checkContentLaneDeliverable #9667 title in the external brief reads 'orb(content-lane): canonicalize...' — worth double-checking the issue link/number is correct since the wording differs slightly from the PR title.
  • src/review/content-lane/registry-logic.ts: consider adding a regression case where the body token uses backslash separators (`Registry\Subnets\Foo.json`) to mirror the existing backslash coverage already present for changed files, since canonicalize() also normalizes `\`→`/`.

Why this is blocked

  • Linked issue's expected content was never delivered: This PR's linked issue names registry/subnets/foo.json, but the PR's changed files never touch it -- the issue's actual content deliverable does not appear to have been added. — Edit registry/subnets/foo.json to deliver the issue's actual ask, or link the correct issue.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. Linked issue's expected content was never delivered: This PR's linked issue names `registry/subnets/foo.json`, but the PR's changed files never touch it -- the issue's actual content deliverable does not appear to have been added. — Edit registry/subnets/foo.json to deliver the issue's actual ask, or link the correct issue.

Decision drivers

  • ❌ Code review — 1 blocker (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #9667
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 251 registered-repo PR(s), 99 merged, 38 issue(s).
Contributor context ✅ Confirmed Gittensor contributor RealDiligent; Gittensor profile; 251 PR(s), 38 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: minor
Linked issue satisfaction

Addressed
The diff exactly implements the required fix (canonicalizing each token before matching while preserving the original token via .find), matching the mirrored classifyRegistryPrScope pattern, and adds tests covering mixed-case missing/delivered, lowercase byte-identical behavior, and the not-over-broadening not-applicable case as required.

Review context
  • Author: RealDiligent
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, Ruby, TypeScript, Svelte, Cuda, JavaScript, Markdown, MDX
  • Official Gittensor activity: 251 PR(s), 38 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

…tLaneDeliverable

checkContentLaneDeliverable compared changed files canonicalized (lowercased,
./-stripped, backslash-normalized) but tested the issue-body path tokens RAW. The
spec patterns are compiled by globToRegExp against a canonicalized path with no `i`
flag, so an issue naming its target file with any capitalization — Registry/Subnets/
Foo.json — produced no mentionedPath, and the deliverable gate silently returned
not-applicable, reopening the "test-only PR closes a content issue without delivering
the content" gap the check exists to close.

Test each token canonicalized at the point of match, mirroring classifyRegistryPrScope's
matchesPattern helper, while find() still returns the ORIGINAL token so the mentionedPath
rendered into the public PR comment quotes the issue body verbatim. Already-lowercase
tokens are byte-identical.

Closes JSONbored#9667
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 29, 2026
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.37%. Comparing base (027e797) to head (17e77fe).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9889      +/-   ##
==========================================
+ Coverage   79.28%   79.37%   +0.09%     
==========================================
  Files         281      282       +1     
  Lines       58510    58786     +276     
  Branches     6777     6895     +118     
==========================================
+ Hits        46387    46663     +276     
  Misses      11840    11840              
  Partials      283      283              
Flag Coverage Δ
backend 100.00% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/review/content-lane/registry-logic.ts 100.00% <100.00%> (ø)

@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (Linked issue's expected content was never delivered). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

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

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

orb(content-lane): canonicalize issue-body path tokens in `checkContentLaneDeliverable

1 participant