diff --git a/.claude/skills/project-setup/SKILL.md b/.claude/skills/project-setup/SKILL.md index 9022fd8..e0e08c5 100644 --- a/.claude/skills/project-setup/SKILL.md +++ b/.claude/skills/project-setup/SKILL.md @@ -45,8 +45,14 @@ to GitHub. creating; use `python3 scripts/gh_project_populate.py docs/GITHUB_PROJECT.md --yes` for non-interactive runs). Issue numbers are written back into the headings as `(#N)` suffixes as each issue is created. -5. **Canonicalize**: `make update` — rewrites the generated regions from - GitHub so the file and GitHub agree exactly from day one. +5. **Canonicalize**: run a plain `make update` once and commit it. + This is a required normalization, not optional polish: update's + canonical rendering drops `**Milestone:**` lines, reorders labels to + GitHub's order, and trims trailing `---`, so the first + `make update-check` on a freshly populated plan is ALWAYS stale + until this run lands. populate prints this sequence as its next-steps + hint. (populate also strips hard line breaks from prose by default + so GitHub soft-wraps it — `--keep-line-breaks` opts out.) ## Safety properties you can rely on diff --git a/.claude/skills/roadmap-upkeep/SKILL.md b/.claude/skills/roadmap-upkeep/SKILL.md index 6b5bd09..5491e3f 100644 --- a/.claude/skills/roadmap-upkeep/SKILL.md +++ b/.claude/skills/roadmap-upkeep/SKILL.md @@ -28,6 +28,12 @@ surrounding prose. Two commands, two directions: Hand-written prose outside the markers is preserved byte-for-byte. Commit the result. +A freshly populated plan's FIRST `make update-check` is always stale — +update's canonical rendering differs cosmetically from authored input +(no `**Milestone:**` lines, labels in GitHub's order, no trailing +`---`). Run one plain `make update`, commit it as the normalization, +and only then treat `update-check` failures as drift. + ## Adding work to a live project 1. New issue in an existing milestone: add a diff --git a/README.md b/README.md index 4efa928..f355169 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,18 @@ that.) Each created issue's number is written back into the file immediately, as a `(#N)` suffix on its heading — commit the result. + By default populate strips hard line breaks from prose before + pushing, so GitHub soft-wraps your issue bodies and milestone + descriptions (`--keep-line-breaks` preserves them verbatim; the run + output states which mode applied). + + Then run a plain `make update` once and commit it as a + normalization: update's canonical rendering differs cosmetically + from authored input (it drops the `**Milestone:**` lines, reorders + labels to GitHub's order, and trims trailing `---` separators), so + the first `make update-check` on a freshly populated plan is + ALWAYS stale — that is normalization pending, not a bug. After the + normalization commit, `update-check` is your drift gate. 6. **From then on**: work the issues on GitHub as usual, and run diff --git a/scripts/VERSION b/scripts/VERSION index 0d91a54..1d0ba9e 100644 --- a/scripts/VERSION +++ b/scripts/VERSION @@ -1 +1 @@ -0.3.0 +0.4.0 diff --git a/scripts/_utils/text_unwrap.py b/scripts/_utils/text_unwrap.py new file mode 100644 index 0000000..0403f92 --- /dev/null +++ b/scripts/_utils/text_unwrap.py @@ -0,0 +1,158 @@ +""" +File: scripts/_utils/text_unwrap.py + +Description: + Remove hard line breaks from markdown prose: every wrapped paragraph + (including list-item continuations) becomes one long line, so renderers that + soft-wrap (the GitHub issue page above all) wrap it themselves. + gh_project_populate.py applies this to the plan file by default, so authored + ~72-column prose reaches GitHub unwrapped. + + Everything that is not reflowable prose survives byte-for-byte: blank lines, + headings, horizontal rules and setext underlines, tables, blockquotes, fenced code + blocks (mermaid, agda, ...), indented code blocks, HTML-comment and generated-region + marker lines, pipe-bearing (potential GFM table) lines, and the plan grammar's + line-oriented bold metadata (`**Labels:** ...`, `**Milestone:** ...`, + `**Repository**: ...`, `**Description:**`) — the engine parses those per line, so + they are structural in every position: nothing joins onto them and they join onto + nothing. + + The acceptance property, pinned by the tests: a plan file parses IDENTICALLY (same + milestones, labels, issue ids and label sets) before and after unwrapping, and + still lints clean; only bodies and descriptions reflow. + + Join rule: a break after sentence-ending punctuation becomes two spaces (the + two-spaces-after-a-period prose style); any other break becomes one space. + `unwrap` is a total function and idempotent. + + Known conservative limitation: a list item's SECOND paragraph (four-space-indented + prose after a blank line inside the item) is indistinguishable from an indented code + block without tracking list context across blank lines, so it is preserved as-is + rather than reflowed. The error is in the safe direction — an unremoved break, + never a changed rendering. + +Design Principles: + - Pure: a single str -> str transform; no I/O (callers use file_ops), no exceptions + on any input. + - Line-classification constants are module-level and independently testable. +""" +from __future__ import annotations + +import re + +# ── Line classification ────────────────────────────────────────────────────── + +FENCE_RE = re.compile(r"^(\s*)(`{3,}|~{3,})") +LIST_ITEM_RE = re.compile(r"^\s*(?:[-*+]|\d{1,9}[.)])\s+\S") +# A bare marker (`*`, `+`, `1.`) is a valid EMPTY list item; joining +# the next line onto it would move that paragraph into the item. +EMPTY_LIST_ITEM_RE = re.compile(r"^\s*(?:[-*+]|\d{1,9}[.)])\s*$") +# `(?:\s|$)`: a bare `#` line is a valid EMPTY heading; requiring +# trailing whitespace would join the next paragraph into it. +HEADING_RE = re.compile(r"^\s{0,3}#{1,6}(?:\s|$)") +# Setext underlines: ANY run of = or - alone on a line (one character +# suffices in CommonMark); joining one into prose would destroy the +# heading above it. +SETEXT_RE = re.compile(r"^\s{0,3}(?:=+|-+)\s*$") +# Thematic breaks: three or more of the SAME marker, spaces/tabs +# permitted between them (`- - -`, `* * *`, `___`). +THEMATIC_RE = re.compile(r"^\s{0,3}([*_-])[ \t]*(?:\1[ \t]*){2,}$") + +# `**Labels:** ...` / `**Milestone:** ...` / `**Repository**: ...` / +# `**Description:**` — the plan grammar is line-oriented for these, so +# such a line is structural in EVERY position: nothing joins onto it +# and it joins onto nothing. (A hand-wrapped metadata VALUE therefore +# stays as authored; unwrap's contract is "never change how the plan +# parses", not "repair wrapped grammar" — lint owns that complaint.) +BOLD_META_RE = re.compile(r"^\s{0,3}\*\*[^*\n]+(?:\*\*\s*:|:\*\*)") + +_SENTENCE_END_RE = re.compile(r"[.!?][\"')\]`*_]*$") + + +def is_structural(line: str) -> bool: + """A line that must never be merged into a neighboring one. + + Any line containing a pipe is protected as a potential GFM table + row — leading pipes are optional in GFM (`Name | Value` over + `--- | ---`), and the cost of a false positive is only an + unremoved break, never a changed rendering. + """ + stripped = line.lstrip() + return bool( + HEADING_RE.match(line) + or SETEXT_RE.match(line) + or THEMATIC_RE.match(line) + or BOLD_META_RE.match(line) + or EMPTY_LIST_ITEM_RE.match(line) + or "|" in line + or stripped.startswith((">", "")) + ) + + +def _joiner(previous: str) -> str: + """Two spaces after a sentence end, one space otherwise.""" + return " " if _SENTENCE_END_RE.search(previous) else " " + + +def _flush(paragraph: list[str], out: list[str]) -> None: + if not paragraph: + return + joined = paragraph[0].rstrip() + for continuation in paragraph[1:]: + joined += _joiner(joined) + continuation.strip() + out.append(joined) + paragraph.clear() + + +def unwrap(text: str) -> str: + """Reflow markdown prose onto single lines; leave structure alone.""" + out: list[str] = [] + paragraph: list[str] = [] + fence_close: re.Pattern[str] | None = None + + for line in text.splitlines(): + if fence_close is not None: + out.append(line) + if fence_close.match(line): + fence_close = None + continue + + opening = FENCE_RE.match(line) + if opening: + _flush(paragraph, out) + out.append(line) + char, count = opening.group(2)[0], len(opening.group(2)) + fence_close = re.compile(rf"^\s*{re.escape(char)}{{{count},}}\s*$") + continue + + if not line.strip(): + _flush(paragraph, out) + out.append(line) + continue + + if paragraph: + if is_structural(line): + _flush(paragraph, out) + out.append(line) + elif LIST_ITEM_RE.match(line): + _flush(paragraph, out) + paragraph.append(line) + else: + paragraph.append(line) + continue + + # Fresh block position: four-space- or tab-indented lines are + # code (list continuations never reach here — their paragraph + # is still open when they arrive). + if line.startswith((" ", "\t")): + out.append(line) + elif is_structural(line): + out.append(line) + else: + paragraph.append(line) + + _flush(paragraph, out) + result = "\n".join(out) + if text.endswith("\n"): + result += "\n" + return result diff --git a/scripts/gh_project_populate.py b/scripts/gh_project_populate.py index 0598507..b861cdf 100755 --- a/scripts/gh_project_populate.py +++ b/scripts/gh_project_populate.py @@ -41,6 +41,9 @@ --issues-only Only create issues (milestones must already exist) --skip-labels Skip label creation --start-from ID Create only issues with ID >= this (e.g. M1-3) + --keep-line-breaks Push bodies exactly as authored; by default hard + line breaks in prose are stripped so GitHub + soft-wraps it --delay SECONDS Pause between mutating calls (default: 1.0) --yes Skip the interactive confirmation --env-prefix Prefix gh with `env -u GH_TOKEN -u GITHUB_TOKEN` @@ -86,6 +89,7 @@ record_issue_number, ) from _utils.file_ops import read_text, write_text # noqa: E402 +from _utils.text_unwrap import unwrap # noqa: E402 # ── Plan reporting (shared verbatim by dry-run and live runs) ─────────────── @@ -323,6 +327,10 @@ def build_parser() -> argparse.ArgumentParser: stage.add_argument("--labels-only", action="store_true") stage.add_argument("--issues-only", action="store_true") parser.add_argument("--skip-labels", action="store_true") + parser.add_argument("--keep-line-breaks", action="store_true", + help="push bodies exactly as authored; by default " + "hard line breaks in prose are stripped so " + "GitHub soft-wraps it") parser.add_argument("--start-from", type=str, default=None, help="Create only issues with ID >= this (e.g. M1-3)") parser.add_argument("--delay", type=float, default=1.0, @@ -360,7 +368,23 @@ def main() -> int: print("error: fix the plan file before populating", file=sys.stderr) return 2 - plan: ProjectPlan = parse_project_plan(text) + # By default, prose is unwrapped before parsing so pushed issue + # bodies and milestone descriptions reach GitHub without authored + # hard line breaks (GitHub soft-wraps). The transform is + # structure-preserving — the plan parses identically, only bodies + # and descriptions reflow (pinned by test_text_unwrap) — and the + # file on disk is never rewritten by it: the (#N) write-back still + # edits the authored text, and the post-populate `update` run is + # what lands normalized content in the file. + if args.keep_line_breaks: + effective = text + print("Line breaks: preserved as authored (--keep-line-breaks)") + else: + effective = unwrap(text) + print("Line breaks: stripped from prose before pushing " + "(use --keep-line-breaks to preserve)") + + plan: ProjectPlan = parse_project_plan(effective) repo = args.repo or plan.repository if repo is None: print( @@ -496,6 +520,27 @@ def main() -> int: f"{issue_failed} failed or skipped, " f"{not_attempted} not attempted" ) + if issues_created and not issue_failed: + # Only after a FULLY successful run: after a partial + # failure this advice would be wrong twice over — the file + # does not yet record every number, and normalizing via + # update from incomplete GitHub state would drop the + # not-yet-created issue definitions from the plan. Failed + # runs keep the per-issue rerun guidance printed above. + # + # A fresh plan's first `make update-check` is ALWAYS stale: + # update's canonical rendering drops the **Milestone:** + # lines, reorders labels to GitHub's order, and trims the + # trailing --- separators. Point at the normalization run + # so that first failure is not read as a bug. + print() + print("Next steps:") + print(" 1. commit this file (it now records the issue numbers)") + print(" 2. run a plain `make update` once and commit the result —") + print(" it normalizes the generated regions to the engine's") + print(" canonical rendering (the first `make update-check` on") + print(" a fresh plan is ALWAYS stale otherwise)") + print(" 3. from then on, `make update-check` is the drift gate") if collisions: print(f"note: {collisions} label collision(s) were skipped — " diff --git a/scripts/tests/test_integration.py b/scripts/tests/test_integration.py index 25fcdea..1363792 100644 --- a/scripts/tests/test_integration.py +++ b/scripts/tests/test_integration.py @@ -338,6 +338,73 @@ def test_update_failure_is_exit_2_not_drift(self) -> None: self.assertIn("no repository", proc.stderr) +class LineBreakHandling(FakeGhHarness): + """Issue #6: populate strips authored hard line breaks by default, + so GitHub soft-wraps prose; --keep-line-breaks opts out. Either + way the mode is stated in the output.""" + + WRAPPED_BODY = "Body of\nM1-1 wrapped over\nthree lines." + WRAPPED_DESC = "Build the\ncore, wrapped." + + def write_wrapped_plan(self) -> None: + self.plan_path.write_text( + PLAN.replace("Body of M1-1.", self.WRAPPED_BODY) + .replace("Build the core.", self.WRAPPED_DESC), + encoding="utf-8", + ) + + def created_m11_body(self) -> str: + issues = self.issues_on_fake_github() + return next(i for i in issues + if i["title"].startswith("[M1-1]"))["body"] + + def test_default_strips_breaks_from_bodies_and_descriptions(self) -> None: + self.write_wrapped_plan() + proc = self.populate() + self.assertEqual(proc.returncode, 0, proc.stdout + proc.stderr) + self.assertIn("Line breaks: stripped from prose", proc.stdout) + + self.assertTrue(self.created_m11_body().startswith( + "Body of M1-1 wrapped over three lines.")) + milestones = json.loads((self.state / "milestones.json").read_text()) + core = next(m for m in milestones if m["title"] == "1. Core") + self.assertTrue(core["description"].startswith( + "Build the core, wrapped.")) + + # Write-back still lands on the authored (wrapped) file. + text = self.plan_path.read_text(encoding="utf-8") + self.assertIn("### Issue M1-1: Set up the build (#", text) + self.assertIn(self.WRAPPED_BODY, text) # file itself untouched + + def test_keep_line_breaks_pushes_verbatim(self) -> None: + self.write_wrapped_plan() + proc = self.populate("--keep-line-breaks") + self.assertEqual(proc.returncode, 0, proc.stdout + proc.stderr) + self.assertIn("Line breaks: preserved as authored", proc.stdout) + self.assertTrue(self.created_m11_body().startswith(self.WRAPPED_BODY)) + + HINT = "fresh plan is ALWAYS stale otherwise" + + def test_next_steps_hint_only_after_fully_successful_runs(self) -> None: + first = self.populate() + self.assertIn(self.HINT, first.stdout) + self.assertIn("`make update`", first.stdout) + self.assertIn("`make update-check`", first.stdout) + rerun = self.populate() + self.assertEqual(rerun.returncode, 0, rerun.stderr) + self.assertNotIn(self.HINT, rerun.stdout) + + def test_no_hint_after_a_partial_failure(self) -> None: + # Normalizing via update from incomplete GitHub state would drop + # the not-yet-created issue definitions from the plan file, so + # a partially failed run must not point users there. + (self.state / "fail_create_issue_at").write_text("2") + proc = self.populate() + self.assertEqual(proc.returncode, 1, proc.stdout + proc.stderr) + self.assertGreater(len(self.issues_on_fake_github()), 0) + self.assertNotIn(self.HINT, proc.stdout) + + class OrganicIssues(FakeGhHarness): """Issue #3 end to end: organically-filed issues render into the `unplanned` region, stay out of populate's way, and cannot inject diff --git a/scripts/tests/test_text_unwrap.py b/scripts/tests/test_text_unwrap.py new file mode 100644 index 0000000..34532d2 --- /dev/null +++ b/scripts/tests/test_text_unwrap.py @@ -0,0 +1,205 @@ +""" +File: scripts/tests/test_text_unwrap.py + +Description: + Tests for _utils.text_unwrap: paragraph reflow semantics, structure + preservation, idempotence, and the acceptance property that matters + to the engine — a plan file parses IDENTICALLY before and after + unwrapping (same milestones, labels, issue ids and label sets) and + still lints clean; only prose reflows. +""" +from __future__ import annotations + +import unittest +from pathlib import Path +import sys + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +from _gh_project_lib import lint_plan, parse_project_plan # noqa: E402 +from _utils.text_unwrap import unwrap # noqa: E402 +from test_gh_project_lib import PLAN # noqa: E402 + +REPO_ROOT = Path(__file__).resolve().parents[2] +EXAMPLE_PLAN = REPO_ROOT / "docs" / "GITHUB_PROJECT.md" + +CASES: tuple[tuple[str, str, str], ...] = ( + ( + "paragraph joins with sentence-aware spacing", + "One line that\nwraps here.\nNext sentence\ncontinues.\n", + "One line that wraps here. Next sentence continues.\n", + ), + ( + "blank lines separate paragraphs", + "First\npara.\n\nSecond\npara.\n", + "First para.\n\nSecond para.\n", + ), + ( + "list items unwrap their continuations but not each other", + "1. **Bold.** Starts\n and continues (x).\n More.\n2. Second\n item.\n", + "1. **Bold.** Starts and continues (x). More.\n2. Second item.\n", + ), + ( + "fenced code is verbatim", + "```mermaid\ngraph TD\n A --> B\n```\nafter\nfence.\n", + "```mermaid\ngraph TD\n A --> B\n```\nafter fence.\n", + ), + ( + "indented code after a blank line is verbatim", + "Prose\nhere.\n\n cmd one\n cmd two\n", + "Prose here.\n\n cmd one\n cmd two\n", + ), + ( + "structural lines never merge", + "# Title\n\nText\nhere.\n\n---\n\n| a | b |\n| - | - |\n\n> quoted\n> lines\n", + "# Title\n\nText here.\n\n---\n\n| a | b |\n| - | - |\n\n> quoted\n> lines\n", + ), + ( + "setext underline is not swallowed into its heading text", + "Title\n---\nbody\ntext.\n", + "Title\n---\nbody text.\n", + ), + ( + "comment and generated-marker lines stay put", + "\n\n\nmirror\n\n", + "\n\n\nmirror\n\n", + ), + ( + "short setext underlines survive (one = or - suffices)", + "Title\n=\nbody\ntext.\n\nSub\n--\nmore\nprose.\n", + "Title\n=\nbody text.\n\nSub\n--\nmore prose.\n", + ), + ( + "spaced thematic breaks survive", + "before.\n- - -\nafter\ntext.\n\n* * *\n\n_ _ _\n", + "before.\n- - -\nafter text.\n\n* * *\n\n_ _ _\n", + ), + ( + "pipe-less GFM tables survive", + "Name | Value\n--- | ---\nfoo | 1\nbar | 2\n\nprose\nafter.\n", + "Name | Value\n--- | ---\nfoo | 1\nbar | 2\n\nprose after.\n", + ), + ( + "tab-indented code is verbatim", + "Prose\nhere.\n\n\tcode one\n\tcode two\n", + "Prose here.\n\n\tcode one\n\tcode two\n", + ), + ( + "a bare # is an empty heading, not a prose prefix", + "#\nbody\ntext.\n\n##\nmore.\n", + "#\nbody text.\n\n##\nmore.\n", + ), + ( + "empty list items stay empty", + "*\nfollowing\npara.\n\n1.\nnumbered\nfollows.\n\n+\nplus too.\n", + "*\nfollowing para.\n\n1.\nnumbered follows.\n\n+\nplus too.\n", + ), + ( + "list second paragraph is conservatively preserved (documented)", + "- item one\n\n second paragraph\n of the item.\n", + "- item one\n\n second paragraph\n of the item.\n", + ), + ( + "valueless bold metadata never absorbs its following prose", + "**Description:**\nBuild the\ncore.\n\n**Exit criterion:**\nCore\nworks.\n", + "**Description:**\nBuild the core.\n\n**Exit criterion:**\nCore works.\n", + ), + ( + "valued bold metadata neither absorbs nor is absorbed", + "**Repository**: `octocat/demo`\nprose right\nafter it.\n\nlead-in prose\n**Labels:** `a`, `b`\n", + "**Repository**: `octocat/demo`\nprose right after it.\n\nlead-in prose\n**Labels:** `a`, `b`\n", + ), + ( + "adjacent bold-metadata lines are not merged", + "### Issue M1-1: Set up\n\n**Labels:** `a`, `b`\n**Milestone:** 1. Core\n\nBody\ntext.\n", + "### Issue M1-1: Set up\n\n**Labels:** `a`, `b`\n**Milestone:** 1. Core\n\nBody text.\n", + ), +) + + +class ReflowSemantics(unittest.TestCase): + def test_cases(self) -> None: + for name, source, expected in CASES: + with self.subTest(name): + self.assertEqual(unwrap(source), expected) + + def test_idempotence(self) -> None: + for name, source, _ in CASES: + with self.subTest(name): + once = unwrap(source) + self.assertEqual(unwrap(once), once) + + def test_total_on_edge_inputs(self) -> None: + self.assertEqual(unwrap(""), "") + self.assertEqual(unwrap("\n"), "\n") + self.assertEqual(unwrap("no newline at eof"), "no newline at eof") + self.assertEqual(unwrap("```\nunclosed fence\nstays put\n"), + "```\nunclosed fence\nstays put\n") + + +class PlanFileAcceptance(unittest.TestCase): + """The property populate relies on: unwrapping changes prose only.""" + + def assert_parses_identically(self, text: str) -> None: + before = parse_project_plan(text) + unwrapped = unwrap(text) + after = parse_project_plan(unwrapped) + self.assertEqual([m.title for m in before.milestones], + [m.title for m in after.milestones]) + self.assertEqual([(l.name, l.color) for l in before.labels], + [(l.name, l.color) for l in after.labels]) + self.assertEqual([(i.id, i.title, i.labels) for i in before.issues], + [(i.id, i.title, i.labels) for i in after.issues]) + self.assertEqual( + [p.message for p in lint_plan(unwrapped) if p.severity == "error"], + [], + ) + + def test_fixture_plan(self) -> None: + self.assert_parses_identically(PLAN) + + @unittest.skipIf(not EXAMPLE_PLAN.exists(), + "full repository tree not available") + def test_shipped_example_plan(self) -> None: + text = EXAMPLE_PLAN.read_text(encoding="utf-8") + self.assert_parses_identically(text) + self.assertIn("```mermaid", unwrap(text)) + + def test_blankless_metadata_shapes_parse_identically(self) -> None: + # Valid plans may omit the blank line after **Description:** / + # **Repository**: — unwrapping must not change what the engine + # extracts from either shape. + variant = PLAN.replace( + "**Description:**\n\nBuild the core.", + "**Description:**\nBuild the core.", + ).replace( + "**Repository**: `octocat/demo`\n", + "**Repository**: `octocat/demo`\nHeader prose follows without a blank.\n", + ) + self.assertNotEqual(variant, PLAN) + before = parse_project_plan(variant) + after = parse_project_plan(unwrap(variant)) + self.assertEqual(before.repository, after.repository) + self.assertEqual(after.repository, "octocat/demo") + self.assertEqual( + [m.description for m in before.milestones], + [m.description for m in after.milestones], + ) + self.assert_parses_identically(variant) + + def test_wrapped_body_reflows(self) -> None: + wrapped = PLAN.replace( + "Body of M2-1.", + "Body of M2-1\nwraps over lines.\nA second sentence\nwraps too.", + ) + self.assertNotEqual(wrapped, PLAN) + self.assert_parses_identically(wrapped) + issues = {i.id: i for i in parse_project_plan(unwrap(wrapped)).issues} + self.assertEqual( + issues["M2-1"].body, + "Body of M2-1 wraps over lines. A second sentence wraps too.", + ) + + +if __name__ == "__main__": + unittest.main()