-
-
Notifications
You must be signed in to change notification settings - Fork 11.6k
wiki: escape brackets in raw article labels (#3547) #3550
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v8
Are you sure you want to change the base?
Changes from 6 commits
09d3d4b
72d3c2d
e030abc
85d2122
ecc031d
235604d
89477d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,42 @@ def _safe_filename(name: str, limit: int = 200) -> str: | |
| return s[:limit] if s else 'unnamed' | ||
|
|
||
|
|
||
| def _escape_md_brackets(text: object) -> str: | ||
| r"""Escape `[`/`]` so raw text embedded in a generated article can never be | ||
| misread as markdown/Obsidian link syntax (#3547). | ||
|
|
||
| A node's own label is source content (an extracted heading, identifier, or | ||
| doc excerpt) and occasionally contains a literal ``[[...]]`` substring — | ||
| e.g. a doc that itself explains or demonstrates wikilink syntax. Printed | ||
| unescaped, that string renders as a real (and always dead — the wiki | ||
| export never writes bracket-style links, see ``_md_link``) wikilink | ||
| instead of the plain text it actually is. | ||
|
|
||
| Callers fall back to a node's own id when it has no ``label`` attribute, | ||
| and a networkx node id is not always a string (an int or a tuple id is | ||
| legal). ``str()`` first so that fallback stringifies exactly like the | ||
| plain f-interpolation this call replaced, instead of raising. | ||
|
|
||
| A backslash immediately before a bracket in the SOURCE text is escaped | ||
| first, before the bracket. Source content occasionally already contains | ||
| a literal backslash right before a bracket (a doc excerpt showing a | ||
| regex character class, ``\]+``, for example). Escaping the bracket alone | ||
| turns that into ``\\]`` — two backslashes then a bare bracket — and | ||
| CommonMark reads a doubled backslash as one literal backslash, which | ||
| un-escapes the bracket right back into live link syntax. Doubling only a | ||
| backslash that precedes a bracket (not every backslash in the text) | ||
| keeps the bracket's own escape intact without touching an unrelated | ||
| pre-existing escape elsewhere in the source (``\*`` meaning a literal | ||
| asterisk, doubled unconditionally, would itself un-escape into a bare, | ||
| newly-live ``*``). | ||
| """ | ||
| return ( | ||
| re.sub(r"\\(?=[\[\]])", r"\\\\", str(text)) | ||
| .replace("[", r"\[") | ||
| .replace("]", r"\]") | ||
| ) | ||
|
|
||
|
|
||
| def _md_link(label: str, resolver: dict[str, str]) -> str: | ||
| """Render a link to another wiki article as a portable relative markdown link. | ||
|
|
||
|
|
@@ -83,7 +119,7 @@ def _md_link(label: str, resolver: dict[str, str]) -> str: | |
| god nodes get article files — render as plain text instead of a dead link | ||
| that points nowhere even inside Obsidian. | ||
| """ | ||
| text = label.replace("[", r"\[").replace("]", r"\]") | ||
| text = _escape_md_brackets(label) | ||
| slug = resolver.get(label) | ||
| if slug is None: | ||
| return text | ||
|
|
@@ -137,7 +173,7 @@ def _community_article( | |
| sources = sorted({G.nodes[n].get("source_file") or "" for n in nodes} - {""}) | ||
|
|
||
| lines: list[str] = [] | ||
| lines += [f"# {label}", ""] | ||
| lines += [f"# {_escape_md_brackets(label)}", ""] | ||
|
|
||
| meta_parts = [f"{len(nodes)} nodes"] | ||
| if cohesion is not None: | ||
|
|
@@ -147,7 +183,7 @@ def _community_article( | |
| lines += ["## Key Concepts", ""] | ||
| for nid in top_nodes: | ||
| d = G.nodes[nid] | ||
| node_label = d.get("label", nid) | ||
| node_label = _escape_md_brackets(d.get("label", nid)) | ||
| src = d.get("source_file", "") | ||
| degree = G.degree(nid) | ||
| src_str = f" — `{src}`" if src else "" | ||
|
|
@@ -185,7 +221,7 @@ def _community_article( | |
| def _god_node_article(G: nx.Graph, nid: str, labels: dict[int, str], node_community: dict[str, int] | None = None, resolver: dict[str, str] | None = None) -> str: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
8 callers depend on it (afferent coupling). Grounded coupling-delta finding (deterministic), not an LLM guess. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
8 callers depend on it (afferent coupling). Grounded coupling-delta finding (deterministic), not an LLM guess. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
8 callers depend on it (afferent coupling). Grounded coupling-delta finding (deterministic), not an LLM guess. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
8 callers depend on it (afferent coupling). Grounded coupling-delta finding (deterministic), not an LLM guess. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
8 callers depend on it (afferent coupling). Grounded coupling-delta finding (deterministic), not an LLM guess. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
8 callers depend on it (afferent coupling). Grounded coupling-delta finding (deterministic), not an LLM guess. |
||
| resolver = resolver or {} | ||
| d = G.nodes[nid] | ||
| node_label = d.get("label", nid) | ||
| node_label = _escape_md_brackets(d.get("label", nid)) | ||
| src = d.get("source_file", "") | ||
| cid = (node_community or {}).get(nid) | ||
| community_name = labels.get(cid, f"Community {cid}") if cid is not None else None | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| """Regression tests for issue #3547: a node label that literally contains | ||
| `[[...]]` (extracted from source content — a doc discussing or demonstrating | ||
| wikilink syntax, for example) must not be printed raw into a generated | ||
| article. `_md_link` already escapes `[`/`]` for anything it links, but three | ||
| other sites print a label directly into the article body without going | ||
| through it: a community's own title, its "Key Concepts" node listing, and a | ||
| god node's own title. Printed unescaped, `[[wikilink]]` renders as a real | ||
| (and always dead — the wiki export never writes bracket-style links) wikilink | ||
| instead of the plain text it actually is. | ||
| """ | ||
| from __future__ import annotations | ||
|
|
||
| import networkx as nx | ||
|
|
||
| from graphify.wiki import _community_article, _god_node_article, _escape_md_brackets, to_wiki | ||
|
|
||
|
|
||
| def test_escape_md_brackets_escapes_both_brackets(): | ||
| assert _escape_md_brackets("[[wikilink]]") == r"\[\[wikilink\]\]" | ||
| assert _escape_md_brackets("plain text") == "plain text" | ||
|
|
||
|
|
||
| def _bracket_is_live_after_escaping(text: str) -> bool: | ||
| """True if unwinding CommonMark backslash-escapes left to right (a `\\X` | ||
| pair consumes both characters and yields one INERT literal `X`) leaves any | ||
| `[`/`]` reachable as a BARE, unpaired character -- i.e. still able to act | ||
| as link syntax rather than literal text.""" | ||
| i = 0 | ||
| while i < len(text): | ||
| if text[i] == "\\" and i + 1 < len(text): | ||
| i += 2 # the pair is consumed together; its second char is inert | ||
| continue | ||
| if text[i] in "[]": | ||
| return True | ||
| i += 1 | ||
| return False | ||
|
|
||
|
|
||
| def test_escape_md_brackets_escapes_a_pre_existing_backslash_before_a_bracket(): | ||
| # A source label can already contain a literal backslash right before a | ||
| # bracket (a doc excerpt showing a regex character class, `\]+`, for | ||
| # example). Escaping the bracket alone would turn it into `\\]` -- CommonMark | ||
| # reads the doubled backslash as one literal backslash, which un-escapes | ||
| # the bracket right back into live syntax. The backslash must be escaped | ||
| # first so the bracket's own escape survives. | ||
| escaped = _escape_md_brackets("regex: \\]+") | ||
| assert escaped == "regex: \\\\\\]+" | ||
| assert not _bracket_is_live_after_escaping(escaped) | ||
|
|
||
|
|
||
| def test_escape_md_brackets_stringifies_a_non_string_node_id_fallback(): | ||
| # A networkx node id is not always a string (int and tuple ids are | ||
| # legal). The label callers fall back to a node's own id when it has no | ||
| # `label` attribute, so this must stringify instead of raising. | ||
| assert _escape_md_brackets(1) == "1" | ||
| assert _escape_md_brackets(("a", "b")) == "('a', 'b')" | ||
|
|
||
|
|
||
| def test_community_article_handles_a_node_with_no_label_and_a_non_string_id(): | ||
| G = nx.Graph() | ||
| G.add_nodes_from([(1, {}), (2, {}), (3, {})]) | ||
| G.add_edges_from([(1, 2, {}), (1, 3, {}), (2, 3, {})]) | ||
| article = _community_article(G, 0, [1, 2, 3], "hello world", {0: "hello world"}, | ||
| None, {1: 0, 2: 0, 3: 0}, {}) | ||
| assert "**1**" in article | ||
|
|
||
|
|
||
| def test_community_title_escapes_bracket_label(): | ||
| G = nx.Graph() | ||
| G.add_node("n1", label="sym", file_type="code", source_file="a.py") | ||
| article = _community_article(G, 0, ["n1"], "[[Foo Bar Baz]]", {0: "[[Foo Bar Baz]]"}, | ||
| None, {"n1": 0}, {}) | ||
| assert "# \\[\\[Foo Bar Baz\\]\\]" in article | ||
| assert "# [[Foo Bar Baz]]" not in article | ||
|
|
||
|
|
||
| def test_community_key_concepts_escapes_node_label(): | ||
| G = nx.Graph() | ||
| G.add_node("n1", label="[[wikilink]]", file_type="concept", source_file="doc.md") | ||
| G.add_node("n2", label="ordinary", file_type="code", source_file="a.py") | ||
| G.add_edge("n1", "n2", relation="related") | ||
| article = _community_article(G, 0, ["n1", "n2"], "Community 0", {0: "Community 0"}, | ||
| None, {"n1": 0, "n2": 0}, {}) | ||
| assert r"\[\[wikilink\]\]" in article | ||
| assert "[[wikilink]]" not in article | ||
|
|
||
|
|
||
| def test_god_node_title_escapes_bracket_label(): | ||
| G = nx.Graph() | ||
| G.add_node("n1", label="[[...]]", file_type="concept", source_file="doc.md") | ||
| G.add_node("n2", label="caller", file_type="code", source_file="a.py") | ||
| G.add_edge("n1", "n2", relation="calls", confidence="EXTRACTED") | ||
| article = _god_node_article(G, "n1", {0: "Community 0"}, {"n1": 0, "n2": 0}, {}) | ||
| assert "# \\[\\[...\\]\\]" in article # only [ and ] are escaped, not the dots | ||
| assert "# [[...]]" not in article | ||
|
|
||
|
|
||
| def test_end_to_end_wiki_export_has_no_literal_wikilinks(tmp_path): | ||
| """A label with content that resembles wikilink placeholder text ("[[link]]", | ||
| "[[new_stem]]", empty "[[]]", per the issue's own examples) must not survive | ||
| verbatim into any generated page.""" | ||
| G = nx.Graph() | ||
| G.add_node("n1", label="[[link]]", file_type="concept", source_file="a.md", community=0) | ||
| G.add_node("n2", label="[[new_stem]]", file_type="concept", source_file="a.md", community=0) | ||
| G.add_node("n3", label="[[]]", file_type="concept", source_file="b.md", community=0) | ||
| G.add_edge("n1", "n2", relation="related") | ||
| G.add_edge("n2", "n3", relation="related") | ||
| communities = {0: ["n1", "n2", "n3"]} | ||
|
|
||
| out = tmp_path / "wiki" | ||
| to_wiki(G, communities, out, community_labels={0: "Community 0"}) | ||
|
|
||
| for md in out.glob("*.md"): | ||
| text = md.read_text(encoding="utf-8") | ||
| assert "[[link]]" not in text, f"{md.name} still has a literal placeholder wikilink" | ||
| assert "[[new_stem]]" not in text, f"{md.name} still has a literal placeholder wikilink" | ||
| assert "[[]]" not in text, f"{md.name} still has a literal empty wikilink" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_god_node_article()8 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.