Skip to content

polish: no double titles, wrapped map labels, export names its format (v0.3.3) - #2

Merged
mabry1985 merged 1 commit into
mainfrom
polish-v0.3.3
Aug 20, 2026
Merged

polish: no double titles, wrapped map labels, export names its format (v0.3.3)#2
mabry1985 merged 1 commit into
mainfrom
polish-v0.3.3

Conversation

@mabry1985

Copy link
Copy Markdown
Member

Three cosmetic items surfaced by the showcase QA run:

  • View: page titles rendered twice — the reader's own heading plus the content's leading H1. The view now strips a leading H1 that matches the page title (a deliberately different H1 is kept).
  • Map: node labels chopped at 22 chars ("Tool Execution Failure…"). Labels now word-wrap to two lines, ellipsizing only past that; the full title rides the node as a native hover tooltip.
  • wiki_export: the tool reply now names which format it wrote ("markdown" or "archive"), so the tutor's confirmation is unambiguous.

102 tests green (2 new for the wrap behavior).

🤖 Generated with Claude Code

… (v0.3.3)

- view: strip a leading H1 that duplicates the page title (a different H1 is kept)
- map: word-wrap node labels to two lines instead of chopping at 22 chars;
  full title rides the node as a hover <title> tooltip
- wiki_export: the tool reply now names which format it wrote

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mabry1985
mabry1985 merged commit 5f5a301 into main Aug 20, 2026
1 check passed
@mabry1985
mabry1985 deleted the polish-v0.3.3 branch August 20, 2026 03:30

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA panel review — WARN

code-review-structural · head 80510572888e · formal

Low-risk PR (title dedup + wiki_export format key): all four findings confirmed, no blockers or majors, nothing refuted — verification changed only the framing, re-reading at branch tip because the head SHA was force-pushed/orphaned (all quotes byte-for-byte). Fix first: view.py:179 — when a body is only a title-matching H1, stripDupTitle returns "" and the || fallback renders the stub placeholder (or a blank body for "# Title\n\n"); the heading still shows as the H2 above, so the invariant to fix is the body-content drop, not the placeholder. Panel agreed on substance; the verifier's nuance is that "silently empty" slightly overstates since the heading still renders. Coverage: no gaps — all three touched files carry findings and the structural pass completed; the real holes are findings 2 and 3 (the format/archive branch and stripDupTitle are both untested).

Findings

Severity Location Finding Verified
🟡 minor view.py:179 A page whose markdown body consists solely of a leading H1 matching its title now renders the "stub — nothing filed yet" placeholder instead of the heading: … confirmed
🟡 minor view.py:132 stripDupTitle — the PR's headline fix for doubled page titles — ships with no test; test_view.py, the suite that asserts against the PAGE string, was not exten… confirmed
🟡 minor tools.py:244 The new format reply key added to wiki_export — the PR's stated purpose — is never asserted by any test, and the archive branch where format="archive" was … confirmed
nit knowledge_map.py:36 _wrap_label's docstring says "only the last line ellipsizes", but the return comprehension ellipsizes any line exceeding width — a non-final line holding a sin… confirmed
findings JSON (machine-readable)
[
  {
    "file": "view.py",
    "line": 179,
    "severity": "minor",
    "category": "removed-behavior",
    "claim": "A page whose markdown body consists solely of a leading H1 matching its title now renders the \"*stub \u2014 nothing filed yet*\" placeholder instead of the heading: stripDupTitle returns \"\" (falsy) so the `||` fallback fires, and real content is silently displayed as empty.",
    "evidence": "+        <div class=\"md\"><h2>${esc(page.title)}</h2>${md(stripDupTitle(page.content_md, page.title) || \"*stub \u2014 nothing filed yet*\")}</div>\n+    return lines.slice(i + 1).join(\"\\n\");",
    "verdict": "confirmed",
    "note": "Re-read at branch tip (head SHA orphaned; retried without ref): both evidence lines verbatim in file. Traced: content \"# Title\" or \"# Title\\n\" -> split, slice(i+1).join -> \"\" (falsy) -> stub fires. Precision: \"# Title\\n\\n\" returns \"\\n\" (truthy) -> renders blank instead of stub; body content is dropped either way. Heading still renders as the H2 above, so 'silently displayed as empty' slightly overstates, but the behavior change and stub-fire are exactly as claimed."
  },
  {
    "file": "view.py",
    "line": 132,
    "severity": "minor",
    "category": "tests",
    "claim": "stripDupTitle \u2014 the PR's headline fix for doubled page titles \u2014 ships with no test; test_view.py, the suite that asserts against the PAGE string, was not extended to pin the dedup behavior or its use in openPage.",
    "evidence": "+  function stripDupTitle(src, title){\n+        <div class=\"md\"><h2>${esc(page.title)}</h2>${md(stripDupTitle(page.content_md, page.title) || \"*stub \u2014 nothing filed yet*\")}</div>",
    "verdict": "confirmed",
    "note": "test_view.py read in full (7 tests, all against PAGE string): no test references stripDupTitle or the dedup call site; no other test file touches the view JS. Both evidence lines verbatim in diff and file."
  },
  {
    "file": "tools.py",
    "line": 244,
    "severity": "minor",
    "category": "tests",
    "claim": "The new `format` reply key added to wiki_export \u2014 the PR's stated purpose \u2014 is never asserted by any test, and the archive branch where `format=\"archive\"` was added has no test coverage at all (test_wiki_export_tool still only checks ok/files).",
    "evidence": "-                return _ok(**res)\n+                return _ok(format=\"archive\", **res)\n-            return _ok(dir=target, files=n)\n+            return _ok(format=\"markdown\", dir=target, files=n)",
    "verdict": "confirmed",
    "note": "All four diff lines verbatim. test_tools.py's test_wiki_export_tool asserts only out['ok'] and out['files'] == 1 (default markdown branch); no test in the repo invokes the wiki_export tool with format=\"archive\". test_archive.py covers store.export_archive directly, but the tool's archive branch and the format reply key are unpinned \u2014 claim accurate."
  },
  {
    "file": "knowledge_map.py",
    "line": 36,
    "severity": "nit",
    "category": "conventions",
    "claim": "_wrap_label's docstring says \"only the last line ellipsizes\", but the return comprehension ellipsizes any line exceeding width \u2014 a non-final line holding a single word wider than LABEL_W is mid-word truncated too, so the comment misstates the behavior.",
    "evidence": "+    \"\"\"Word-wrap a node label to at most max_lines; only the last line ellipsizes.\"\"\"\n+    return [ln if len(ln) <= width else ln[: width - 1] + \"\u2026\" for ln in lines if ln]",
    "verdict": "confirmed",
    "note": "Both evidence lines verbatim in file. Traced char-by-char: title 'supercalifragilisticexpialidocious x' (word len 34 > 26) -> first word set via 'not cur' branch, next word pushes it into lines as a NON-final line -> comprehension mid-word truncates it (ln[:25]+'\u2026'). A non-final line does ellipsize, so the docstring misstates. Nit severity correct."
  }
]

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant