Fix false local references from slash-separated prose - #451
Conversation
Signed-off-by: Tomas <tomas.bitvinskas@copla.com>
|
End-to-end follow-up:
This confirms the parser change at the installed consumer boundary. |
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
The prose false positives are removed, but the new explicit-path branch also drops valid root-level extensionless references such as ./guide. Please retain that existing behavior and add the missing boundary regression. All required checks are green.
|
|
||
| _PLAIN_RELATIVE_PATH = re.compile( | ||
| r"(?<![\w:/.-])((?:\./)?(?:[A-Za-z0-9_.-]+/)+[A-Za-z0-9_.-]+(?:\.[A-Za-z0-9]{1,12})?)(?![\w/.-])" | ||
| r"(?<![\w:/.-])((?:\./(?:[A-Za-z0-9_.-]+/)+[A-Za-z0-9_.-]+|" |
There was a problem hiding this comment.
[P2] Preserve root-level extensionless ./ references. This branch requires at least one additional directory component after the ./ prefix, so ./guide no longer matches even though an explicit ./ is the syntax signal promised by the issue/PR (and the old regex did match it). Allow zero or more directory segments after ./ and add ./guide to the regression matrix.
There was a problem hiding this comment.
Addressed in 2bc2c12. The explicit ./ branch now permits zero directory segments, and the regression matrix includes ./guide resolving to guide. The unprefixed slash-prose branch is unchanged.
Verification: the exact test failed before the fix and now passes 3/3; the focused resolver set passes 10/10; Ruff and git diff --check pass. All five upstream checks are green.
Context: Plain reference matching required a directory segment after an explicit `./`, so `./guide` produced no candidate even though the prefix disambiguates the path from prose. Changes: - Allow zero or more directory segments between `./` and the final path component. - Add the root-level extensionless case to the existing explicit-path regression matrix. Impact: Root-level extensionless references with an explicit `./` resolve again. Unprefixed slash-separated prose remains excluded, and nested explicit paths keep their existing behavior. Validation: - `uv run --extra dev pytest -q tests/nodes/test_security_remediation.py::test_plain_local_reference_requires_an_explicit_path_signal`: 3 passed. - Focused reference-resolver set: 10 passed. - Ruff on the changed source and test files: passed. - `git diff --check`: passed. Notes: None. Signed-off-by: Tomas <180413002+Tomauskasz@users.noreply.github.com>
Context: Issue NVIDIA#450 lists model/provider as ordinary slash-separated prose that must not become a local file reference. Changes: - Add model/provider to the existing plain-prose reference regression case. Impact: Extend the false-positive contract coverage without changing production reference parsing. Validation: - Focused reference suite: 14 passed, 57 deselected. - Lint: All checks passed. - Format check: 193 files already formatted. - Build: source distribution and wheel built successfully. - git diff --check: passed. Notes: Bare dotted tokens remain excluded because versions, domains, and identifiers are ambiguous with root-level filenames. Signed-off-by: Tomas <180413002+Tomauskasz@users.noreply.github.com>
Context: Bare dotted tokens are ambiguous with root-level filenames and can fabricate missing references for versions, domains, or ecosystem names. Changes: - Add SkillSpector 2.10.0, node.js, and example.com to the plain-prose negative reference case. Impact: Protect the partial-analysis boundary from a future broad match of unquoted dotted prose. Validation: - Focused reference suite: 14 passed, 57 deselected. - Ruff check: All checks passed. - git diff --check: passed. Notes: Root-level filenames remain available through quotes, code spans, Markdown links, or an explicit ./ prefix. Signed-off-by: Tomas <180413002+Tomauskasz@users.noreply.github.com>
|
Final review polish is in f9c2cf1. The focused regression now also proves that version strings, domains, and ecosystem names such as Final verification: 2,956 passed, 14 skipped, 38 deselected, 4 xfailed; Ruff lint and format checks passed; source and wheel builds passed; |
3 participants
Summary
reads/writesandmodel/providerduring plain-text reference extraction../guide.mdand./references/guide.references/guide.md, and existing quoted, code-span, and Markdown destinations.Fixes #450.
Root cause
The plain relative-path pattern accepted a multi-segment token when both the explicit
./prefix and final file extension were absent. The resolver therefore could not distinguish ordinary prose from an extensionless local path.Unquoted plain-text references now require an explicit
./prefix, or both a directory segment and final extension. Bare dotted tokens remain excluded because versions such as2.10.0, domains such asexample.com, and ecosystem names such asnode.jsare indistinguishable from root-level filenames without another path signal.Root-level files remain expressible through
./guide.md, quotes or code spans, or Markdown link syntax.Verification
14 passed, 57 deselected2956 passed, 14 skipped, 38 deselected, 4 xfailedruff check: passedruff format --check: passedgit diff --check: passedTradeoff
An unquoted extensionless token such as
references/guide, or a bare dotted token such asguide.md, is ambiguous with prose. Authors must use an explicit./prefix, quotes or a code span, or Markdown link syntax. Nested extension-bearing paths such asreferences/guide.mdremain supported.AI assistance disclosure
Codex inspected the reference resolver, reproduced the defect, tested version, domain, ecosystem-name, and slash-prose counterexamples, drafted the implementation and tests, and ran the verification commands. I reviewed the root cause, conservative contract, diff, tradeoff, and final pull-request text.