-
-
Notifications
You must be signed in to change notification settings - Fork 11.5k
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 all 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,48 @@ 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 RUN of one or more backslashes immediately before a bracket in the | ||
| SOURCE text is doubled first, before the bracket is escaped. Source | ||
| content occasionally already contains one or more literal backslashes | ||
| right before a bracket (a doc excerpt showing a regex character class, | ||
| ``\]+``, or an escaped backslash inside one, ``[\\]``, for example). | ||
| Doubling only the LAST backslash of a run, rather than every backslash | ||
| in the run, still leaves the earlier ones unpaired: two source | ||
| backslashes before a bracket produced four backslashes then a bare | ||
| bracket, and CommonMark reads two backslash pairs as two literal | ||
| backslashes with nothing left to escape the bracket, un-escaping it | ||
| right back into live link syntax. The whole run must double, then get | ||
| one more backslash for the bracket's own escape, in a single pass — | ||
| two separate ``.replace()`` calls for the brackets can't do this, since | ||
| the second call has no way to know which backslashes a previous | ||
| replacement result already produced versus which were in the run. | ||
| Matching (and touching) only a run that actually precedes a bracket | ||
| keeps an unrelated pre-existing escape elsewhere in the source intact | ||
| (``\*`` meaning a literal asterisk, doubled unconditionally, would | ||
| itself un-escape into a bare, newly-live ``*``). | ||
| """ | ||
| def _double_and_escape(m: "re.Match[str]") -> str: | ||
| return m.group(1) * 2 + "\\" + m.group(2) | ||
|
|
||
| return re.sub(r"(\\*)([\[\]])", _double_and_escape, str(text)) | ||
|
|
||
|
|
||
| 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 +125,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 +179,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 +189,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 +227,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.