Skip to content

fix(content-lane): canonicalize the issue-body path token in checkContentLaneDeliverable - #9788

Closed
shin-core wants to merge 1 commit into
JSONbored:mainfrom
shin-core:fix/deliverable-canonicalize-token-9667
Closed

fix(content-lane): canonicalize the issue-body path token in checkContentLaneDeliverable#9788
shin-core wants to merge 1 commit into
JSONbored:mainfrom
shin-core:fix/deliverable-canonicalize-token-9667

Conversation

@shin-core

Copy link
Copy Markdown
Contributor

What & why

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 compared each issue-body token against matchesSpec raw:

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

const delivered = changedFiles.some((file) => matchesSpec(canonicalize(file)));   // canonicalized

entryFilePattern/providerFilePattern are compiled by globToRegExp, which canonicalizes (lowercases) the glob and emits ^…$ with no i flag — so the patterns only ever match lowercase input. An issue body naming its path with any capitalization (Registry/Subnets/Foo.json) produced no mentionedPath, and with no title fallback the check returned not-applicable — the gate silently didn't run, exactly the gap the function exists to close. The changedFiles side already canonicalizes; the issue-body side never got that treatment (classifyRegistryPrScope in the same file documents why it matters).

The fix

Canonicalize the token at the point of test, mirroring classifyRegistryPrScope's matchesPattern:

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

mentionedPath still holds the original token — it is quoted verbatim into the public PR comment, so it must match what the contributor and maintainer see in the issue. Already-lowercase tokens are byte-identical to today. No change to extractPathTokens, globToRegExp, canonicalize, or the issueTitleImpliesEntryPattern fallback (no i flag added to the compiled globs).

Tests (test/unit/content-lane-registry-logic.test.ts)

New #9667 suite:

  • Mixed-case issue path (Registry/Subnets/Foo.json) against a non-delivering PR → missing with mentionedPath quoting the original mixed-case token (fails on main).
  • Same mixed-case issue path, PR changes registry/subnets/foo.jsondelivered (fails on main).
  • Already-lowercase behaviour unchanged (both delivered and missing).
  • An issue path matching no spec pattern still returns not-applicable (not over-broadened).

Validation

  • npm run typecheck green; the suite (175 tests) green.
  • Diff coverage on the changed file is 100% line and branch; whole-file branch 100%.
  • git diff --check <base> HEAD clean; diff is two files, no route/schema/migration change.

Closes #9667

…tentLaneDeliverable

`checkContentLaneDeliverable` tested each `extractPathTokens` token against
`matchesSpec` RAW, but the spec patterns are compiled by `globToRegExp` against a
canonicalized (lowercased) path with no `i` flag — so they only match lowercase
input. An issue body naming its target with any capitalization
(`Registry/Subnets/Foo.json`) produced no `mentionedPath`, and the deliverable
gate silently returned `not-applicable` — the exact "test-only PR closes a
content issue without delivering the content" gap this check exists to close. The
`changedFiles` side already canonicalizes; the issue-body side never did.

Canonicalize the token at the point of test (mirroring `classifyRegistryPrScope`'s
`matchesPattern`) while keeping the ORIGINAL token for `mentionedPath`, which is
quoted verbatim into the public PR comment. Already-lowercase paths are unchanged.

Closes JSONbored#9667
@shin-core
shin-core requested a review from JSONbored as a code owner July 29, 2026 08:42
@superagent-security superagent-security Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Jul 29, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

@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 08:53:22 UTC

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

🛑 Suggested Action - Reject/Close

Review summary
This is a narrow, correct one-line fix: `checkContentLaneDeliverable` now tests `matchesSpec(canonicalize(token))` against each extracted issue-body path token instead of the raw token, mirroring `classifyRegistryPrScope`'s already-established `matchesPattern` pattern for the same canonicalization gap (globToRegExp lowercases the compiled pattern but the raw issue token wasn't canonicalized before testing). `mentionedPath` is deliberately kept as the original (non-canonicalized) token since it's quoted verbatim into the public PR comment. The accompanying tests exercise mixed-case delivered/missing/not-applicable cases and confirm already-lowercase behavior is unchanged, directly covering the new branch.

Nits — 3 non-blocking
  • The inline comment on registry-logic.ts:933-936 is fairly long for a one-line change; consider trimming since the file's module-level comments already explain canonicalize/globToRegExp semantics.
  • No test exercises the `titleImplies` fallback path in combination with a mixed-case scenario, though this is a minor gap given the fix is orthogonal to that branch.
  • Consider a shorter inline comment pointing to `classifyRegistryPrScope`'s `matchesPattern` for the full canonicalization rationale rather than restating it.

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 ❌ 5/25 Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload ✅ 10/10 Author activity: 50 registered-repo PR(s), 35 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor shin-core; Gittensor profile; 50 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: minor
Linked issue satisfaction

Addressed
The diff makes the exact one-line change requested — canonicalizing the token at the point of test via matchesSpec(canonicalize(token)) while keeping mentionedPath as the original — and adds tests covering mixed-case missing/delivered, lowercase-unchanged behavior, and a no-match not-applicable case, matching all four Deliverables.

Review context
  • Author: shin-core
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is registered but has no active allocation in the current snapshot.
  • Public profile languages: not available
  • Official Gittensor activity: 50 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Await review-lane availability.
  • Then work through the remaining 2 steps in the Signals table above.
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 &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; 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.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 29, 2026
@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.

@loopover-orb loopover-orb Bot closed this 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 76.69%. Comparing base (0fa52ed) to head (85c15d2).

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #9788       +/-   ##
===========================================
- Coverage   90.30%   76.69%   -13.61%     
===========================================
  Files         913      283      -630     
  Lines      113595    59740    -53855     
  Branches    26963     6673    -20290     
===========================================
- Hits       102584    45819    -56765     
- Misses       9682    13639     +3957     
+ Partials     1329      282     -1047     
Flag Coverage Δ
backend 100.00% <100.00%> (+4.43%) ⬆️

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%> (ø)

... and 764 files with indirect coverage changes

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

Labels

contributor:flagged Contributor flagged for review by trust analysis. 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