mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(skills): strip section anchors from code-span resource refs (#5841)
* fix(skills): strip section anchors from code-span resource refs Markdown-link targets already drop their #fragment before the reference is resolved, but code-span refs did not: a prose reference like `references/faq.md#pricing` kept the anchor and was resolved as a literal path, producing a resource.missing finding for a file that exists and is referenced. The bare-path token scanner stops at '#', so only the code-span branch leaked fragments. Split the fragment off code-span tokens the same way the link branch does, then strip trailing sentence punctuation. Real dotted filenames keep their dots: the split happens at the first '#', and '.' is only stripped from the very end. Signed-off-by: yetuge <2219677952@qq.com> * fix(skills): prefer exact hash-bearing filenames over fragment stripping Package paths permit '#', so a code-span reference to a real file like references/C#.md was truncated to references/C by the fragment split, producing both a false resource.missing and an orphan report (the bare path token stopped at '#' the same way). Keep the whole reference in every extraction branch (link target, code span, bare path token now includes '#' in its character class) and resolve the fragment existence-aware: prefer the exact token when it names a real file and treat the suffix after the first '#' as a section fragment only when it does not. Red/green: a new test pins references/C#.md resolving to the real file while references/faq.md#pricing still strips to faq.md — it fails on the reviewed head and passes with the change; full test_skill_review_core.py is 36 passed. Signed-off-by: yetuge <2219677952@qq.com> * fix(skills): strip the fragment before normalizing the fallback path The exact-path preference checked the normalized reference, so a token like references/faq.md#/../other.md normalized to references/other.md; when that file existed the check succeeded and the fragment-removal branch was skipped, recording the wrong edge and orphaning faq.md. Check the literal token against the package files instead, and drop the fragment before the remaining path is normalized — normalizing a hash-bearing segment must never retarget an edge. Signed-off-by: yetuge <2219677952@qq.com> * fix(skills): canonicalize leading segments in the hash-filename lookup The literal-file check compared the verbatim joined path, whose leading relative segments are uncanonicalized: from references/sub/guide.md, a reference to ../C#.md joins to references/sub/../C#.md, matches no snapshot key, and falls through to fragment stripping — truncating the real references/C#.md into a false missing and an orphan. Compare against the normpath-canonicalized whole token instead, and treat the post-'#' text as a fragment whenever it is path-like (carries / or ..) so it can still never participate in normalization and retarget an edge. Signed-off-by: yetuge <2219677952@qq.com> * fix(skills): check the canonical whole path before stripping a fragment The path-like-fragment heuristic dropped any post-'#' text carrying '/': a real package file such as references/C#/readme.md (a '#' inside a directory name) was truncated to references/C, producing a false resource.missing and orphaning readme.md. Only a '..' segment in the post-'#' text forces the strip-first fallback — that is the one case where fragment text participates in normalization and can retarget an edge (faq.md#/../other.md). Every other fragment first gets the canonicalized whole-token lookup, which already canonicalizes leading relative segments for nested sources. Signed-off-by: yetuge <2219677952@qq.com> * fix(skills): always strip markdown link fragments and blind the bare-path pass to links Markdown link targets have fixed fragment semantics: the text after '#' is a URL fragment, never part of the filename, so a link to a file literally named references/faq.md#pricing would have to percent-encode it. The exact-file branch applied the hash-filename preference to link targets too, recording a valid edge and suppressing resource.missing for a broken link when a literal same-named file existed. Links strip the fragment again, and their full construct is blanked out of the residual text so the code-span and bare-path passes — literal-path contexts where '#' may be part of a real filename — never see link-internal text. Signed-off-by: yetuge <2219677952@qq.com> --------- Signed-off-by: yetuge <2219677952@qq.com>
This commit is contained in:
yetuge
GitHub
parent
b2a2fb757a
commit
b4a60fbfe2
@ -2,7 +2,9 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import posixpath
|
||||
import re
|
||||
from collections.abc import Set as AbstractSet
|
||||
from pathlib import PurePosixPath
|
||||
from typing import Any
|
||||
|
||||
@ -11,7 +13,7 @@ from deerflow.skills.review.models import make_finding, normalize_relative_path
|
||||
|
||||
_MARKDOWN_LINK_RE = re.compile(r"!?\[[^\]]*]\(([^)\s]+)(?:\s+\"[^\"]*\")?\)")
|
||||
_CODE_SPAN_RE = re.compile(r"`([^`]+)`")
|
||||
_PATH_TOKEN_RE = re.compile(r"(?<![\w./-])(?:references|scripts|templates|assets|evals)/[A-Za-z0-9._~/%+-]+")
|
||||
_PATH_TOKEN_RE = re.compile(r"(?<![\w./-])(?:references|scripts|templates|assets|evals)/[A-Za-z0-9._~/%+#-]+")
|
||||
_RESOURCE_DIRS = {"references", "scripts", "templates", "assets", "evals"}
|
||||
|
||||
|
||||
@ -29,7 +31,7 @@ def build_resource_graph(snapshot: dict[str, Any]) -> tuple[dict[str, Any], list
|
||||
continue
|
||||
content = str(entry.get("content") or "")
|
||||
for raw_ref in _extract_references(content):
|
||||
resolved = _resolve_reference(path, raw_ref)
|
||||
resolved = _resolve_reference(path, raw_ref, files.keys())
|
||||
if resolved is None:
|
||||
continue
|
||||
if resolved == "__ESCAPES__":
|
||||
@ -96,18 +98,32 @@ _TRAILING_SENTENCE_PUNCTUATION = ".?!"
|
||||
|
||||
def _extract_references(content: str) -> set[str]:
|
||||
refs: set[str] = set()
|
||||
# Markdown link syntax fixes the fragment semantics: the text after '#'
|
||||
# in a link target is ALWAYS a URL fragment, never part of the filename
|
||||
# — a link to a file literally named "faq.md#pricing" would have to
|
||||
# percent-encode it. So links always strip the fragment, and their full
|
||||
# construct is blanked out of the residual text: the code-span and
|
||||
# bare-path passes below are literal-path contexts where '#' may be
|
||||
# part of a real filename, and they must never see link-internal text.
|
||||
# Trailing sentence punctuation is still stripped here (#5739).
|
||||
residual = content
|
||||
for match in _MARKDOWN_LINK_RE.finditer(content):
|
||||
refs.add(match.group(1).split("#", 1)[0].rstrip(_TRAILING_SENTENCE_PUNCTUATION))
|
||||
for match in _CODE_SPAN_RE.finditer(content):
|
||||
residual = residual.replace(match.group(0), " " * len(match.group(0)))
|
||||
for match in _CODE_SPAN_RE.finditer(residual):
|
||||
token = match.group(1).strip()
|
||||
if "/" in token:
|
||||
refs.add(token.rstrip(_TRAILING_SENTENCE_PUNCTUATION))
|
||||
for match in _PATH_TOKEN_RE.finditer(content):
|
||||
for match in _PATH_TOKEN_RE.finditer(residual):
|
||||
refs.add(match.group(0).rstrip(_TRAILING_SENTENCE_PUNCTUATION))
|
||||
return refs
|
||||
|
||||
|
||||
def _resolve_reference(source_path: str, raw_ref: str) -> str | None:
|
||||
def _resolve_reference(
|
||||
source_path: str,
|
||||
raw_ref: str,
|
||||
files: AbstractSet[str] | None = None,
|
||||
) -> str | None:
|
||||
ref = raw_ref.strip().strip("\"'")
|
||||
if not ref or ref.startswith("#") or re.match(r"^[A-Za-z][A-Za-z0-9+.-]*:", ref):
|
||||
return None
|
||||
@ -117,7 +133,23 @@ def _resolve_reference(source_path: str, raw_ref: str) -> str | None:
|
||||
base = PurePosixPath(source_path).parent
|
||||
if "://" in ref:
|
||||
return None
|
||||
candidate = (base / ref).as_posix()
|
||||
return normalize_relative_path(candidate)
|
||||
if "#" in ref:
|
||||
pre_hash, fragment = ref.split("#", 1)
|
||||
# A '..' segment in the post-'#' text must never participate in
|
||||
# normalization: it would collapse the hash-bearing segment and
|
||||
# retarget the edge ("faq.md#/../other.md" must resolve to
|
||||
# faq.md). Any other fragment gets its chance first: the whole
|
||||
# token may name a real package file — '#' is legal in file AND
|
||||
# directory names ("references/C#.md", "references/C#/readme.md"),
|
||||
# from nested sources too ("../C#.md"). Only when the
|
||||
# canonicalized whole token matches no file does the suffix
|
||||
# become a section fragment and get stripped.
|
||||
if "/.." in f"/{fragment}" or fragment.startswith(".."):
|
||||
ref = pre_hash
|
||||
elif files is not None and posixpath.normpath((base / ref).as_posix()) in files:
|
||||
return normalize_relative_path((base / ref).as_posix())
|
||||
else:
|
||||
ref = pre_hash
|
||||
return normalize_relative_path((base / ref).as_posix())
|
||||
except ValueError:
|
||||
return "__ESCAPES__"
|
||||
|
||||
@ -152,6 +152,133 @@ def test_resource_graph_keeps_real_dotted_filenames(tmp_path):
|
||||
assert not any(f["rule_id"] == "resource.missing" and f["path"] == "SKILL.md" for f in facts["findings"])
|
||||
|
||||
|
||||
def test_resource_graph_strips_fragment_from_code_span_refs(tmp_path):
|
||||
# A code span can carry a section anchor just like a markdown link
|
||||
# target ("`references/faq.md#pricing`"). The anchor is not part of
|
||||
# the path: it must not turn a valid reference into a
|
||||
# resource.missing finding.
|
||||
_write(
|
||||
tmp_path / "SKILL.md",
|
||||
_valid_skill() + "\nSee `references/faq.md#pricing` for details.\n",
|
||||
)
|
||||
_write(tmp_path / "references" / "faq.md", "# FAQ\n")
|
||||
|
||||
facts = analyze_skill_package(LocalDirectoryReader(tmp_path).read())
|
||||
|
||||
assert {"source": "SKILL.md", "target": "references/faq.md"} in facts["resources"]["edges"]
|
||||
assert "references/faq.md" not in facts["resources"]["orphans"]
|
||||
assert not any(f["rule_id"] == "resource.missing" and f["path"] == "SKILL.md" for f in facts["findings"])
|
||||
|
||||
|
||||
def test_resource_graph_keeps_real_dots_when_stripping_code_span_fragments(tmp_path):
|
||||
# Stripping the fragment must not eat real dotted filenames: the
|
||||
# extension dots of a fragment-bearing code span reference survive.
|
||||
_write(
|
||||
tmp_path / "SKILL.md",
|
||||
_valid_skill() + "\nRead `references/v1.0.md#notes`.\n",
|
||||
)
|
||||
_write(tmp_path / "references" / "v1.0.md", "# v1.0\n")
|
||||
|
||||
facts = analyze_skill_package(LocalDirectoryReader(tmp_path).read())
|
||||
|
||||
assert {"source": "SKILL.md", "target": "references/v1.0.md"} in facts["resources"]["edges"]
|
||||
assert not any(f["rule_id"] == "resource.missing" and f["path"] == "SKILL.md" for f in facts["findings"])
|
||||
|
||||
|
||||
def test_resource_graph_prefers_hash_filenames_over_fragments(tmp_path):
|
||||
# A package filename may legally contain '#': a code-span reference to
|
||||
# `references/C#.md` must resolve to the real file, not be truncated to
|
||||
# `references/C` by fragment stripping. The suffix is only treated as a
|
||||
# fragment when the exact path does not exist.
|
||||
_write(
|
||||
tmp_path / "SKILL.md",
|
||||
_valid_skill() + "\nSee `references/C#.md` and `references/faq.md#pricing`.\n",
|
||||
)
|
||||
_write(tmp_path / "references" / "C#.md", "# C#\n")
|
||||
_write(tmp_path / "references" / "faq.md", "# FAQ\n")
|
||||
|
||||
facts = analyze_skill_package(LocalDirectoryReader(tmp_path).read())
|
||||
|
||||
assert {"source": "SKILL.md", "target": "references/C#.md"} in facts["resources"]["edges"]
|
||||
assert {"source": "SKILL.md", "target": "references/faq.md"} in facts["resources"]["edges"]
|
||||
assert not any(f["rule_id"] == "resource.missing" and f["path"] == "SKILL.md" for f in facts["findings"])
|
||||
|
||||
|
||||
def test_resource_graph_prefers_hash_filenames_from_nested_sources(tmp_path):
|
||||
# The literal-file check must canonicalize leading relative segments
|
||||
# before comparing against the snapshot keys: from
|
||||
# references/sub/guide.md, `../C#.md` joins to
|
||||
# `references/sub/../C#.md`, which matches no key verbatim — without
|
||||
# canonicalization the reference is truncated to `../C` and produces a
|
||||
# false resource.missing plus an orphan report for the real file.
|
||||
_write(tmp_path / "SKILL.md", _valid_skill())
|
||||
_write(
|
||||
tmp_path / "references" / "sub" / "guide.md",
|
||||
_valid_skill("guide") + "\nSee `../C#.md`.\n",
|
||||
)
|
||||
_write(tmp_path / "references" / "C#.md", "# C#\n")
|
||||
|
||||
facts = analyze_skill_package(LocalDirectoryReader(tmp_path).read())
|
||||
|
||||
assert {"source": "references/sub/guide.md", "target": "references/C#.md"} in facts["resources"]["edges"]
|
||||
assert not any(f["rule_id"] in {"resource.missing", "resource.escaping-link"} and f["path"] == "references/sub/guide.md" for f in facts["findings"])
|
||||
|
||||
|
||||
def test_resource_graph_prefers_hash_directory_paths_over_stripping(tmp_path):
|
||||
# '#' is legal in directory names too: the whole token
|
||||
# `references/C#/readme.md` names a real file even though the text
|
||||
# after '#' contains '/'. It must not be truncated to `references/C`
|
||||
# — only a '..' segment in the post-'#' text forces the strip-first
|
||||
# fallback ("faq.md#/../other.md").
|
||||
_write(
|
||||
tmp_path / "SKILL.md",
|
||||
_valid_skill() + "\nSee `references/C#/readme.md`.\n",
|
||||
)
|
||||
_write(tmp_path / "references" / "C#" / "readme.md", "# C#\n")
|
||||
|
||||
facts = analyze_skill_package(LocalDirectoryReader(tmp_path).read())
|
||||
|
||||
assert {"source": "SKILL.md", "target": "references/C#/readme.md"} in facts["resources"]["edges"]
|
||||
assert not any(f["rule_id"] == "resource.missing" and f["path"] == "SKILL.md" for f in facts["findings"])
|
||||
|
||||
|
||||
def test_resource_graph_strips_fragment_before_normalizing_fallback(tmp_path):
|
||||
# The exact-path preference must check the literal token: normalizing
|
||||
# first collapses a hash-bearing segment ("faq.md#/.." -> "other.md")
|
||||
# and can silently retarget the edge and orphan faq.md. The fragment is
|
||||
# dropped first when the literal token is not a real file.
|
||||
_write(
|
||||
tmp_path / "SKILL.md",
|
||||
_valid_skill() + "\nSee `references/faq.md#/../other.md`.\n",
|
||||
)
|
||||
_write(tmp_path / "references" / "faq.md", "# FAQ\n")
|
||||
_write(tmp_path / "references" / "other.md", "# Other\n")
|
||||
|
||||
facts = analyze_skill_package(LocalDirectoryReader(tmp_path).read())
|
||||
|
||||
assert {"source": "SKILL.md", "target": "references/faq.md"} in facts["resources"]["edges"]
|
||||
assert not any(f["rule_id"] == "resource.missing" and f["path"] == "SKILL.md" for f in facts["findings"])
|
||||
|
||||
|
||||
def test_resource_graph_always_strips_markdown_link_fragments(tmp_path):
|
||||
# In Markdown link syntax the text after '#' is always a URL fragment —
|
||||
# a link to a file literally named "faq.md#pricing" would have to
|
||||
# percent-encode it. The link must resolve to faq.md and stay broken
|
||||
# (resource.missing) even when a file literally named
|
||||
# references/faq.md#pricing exists; the bare-path pass must not see the
|
||||
# link-internal text and resurrect the literal edge.
|
||||
_write(
|
||||
tmp_path / "SKILL.md",
|
||||
_valid_skill() + "\nSee [FAQ](references/faq.md#pricing).\n",
|
||||
)
|
||||
_write(tmp_path / "references" / "faq.md#pricing", "# trap\n")
|
||||
|
||||
facts = analyze_skill_package(LocalDirectoryReader(tmp_path).read())
|
||||
|
||||
assert not any(e["target"] == "references/faq.md#pricing" for e in facts["resources"]["edges"])
|
||||
assert any(f["rule_id"] == "resource.missing" and f["path"] == "SKILL.md" and "references/faq.md" in f["message"] for f in facts["findings"])
|
||||
|
||||
|
||||
def test_resource_graph_ignores_eval_fixture_references(tmp_path):
|
||||
_write(tmp_path / "SKILL.md", _valid_skill())
|
||||
_write(
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user