From f07369776213b2e76eab6b820d93232efcdf0720 Mon Sep 17 00:00:00 2001 From: DemchaAV Date: Fri, 31 Jul 2026 19:46:06 +0100 Subject: [PATCH 1/2] test(guards): fail the build on the documentation drift this line kept re-fixing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four releases of hand-corrections shared one property: nothing would have caught any of them. The guards below close that, each one keyed to a defect that actually shipped. Link liveness reads what the documents point at, so it needs no token list and cannot go stale. It found nine dead source links in implementation-guide.md on its first run. Retired-surface scanning covers README, CONTRIBUTING, SECURITY, SUPPORT, ROADMAP, the examples gallery, docs/ and .github/ — the last four sat outside every existing guard, which is how an issue template could route reporters to a theme class removed in 2.0. The token list is what verification supports: seven types absent from every src/main tree. BusinessTheme and PptxSemanticBackend are deliberately absent from it — both are alive, in 21 and 2 source files, and forbidding them would fail the build on correct text. Historical records are skipped by path prefix rather than a file allowlist, so a new page under docs/archive/ is covered the day it lands. DocsBoldFaceGuardTest pins the font rule: a *_BOLD constant resolves to its base family and the face comes from the decoration, so naming the alias without one renders regular. Its self-check counts every font selection rather than every alias — counting aliases made a clean tree look like a broken scan, because the correct end state has none. The release-status assertion requires a published version and requires the link to point at the tag the text names, so a half-updated block cannot pass. implementation-guide.md loses the two sections built on the execution layer 2.0 removed, and the render-contract references that went with them: Render, RenderPassSession, RenderStream, TextComponent and BlockText are absent from every src/main tree. Deleting them makes the guide less wrong; describing the real pipeline in their place is still open. --- CHANGELOG.md | 9 ++ .../CanonicalSurfaceGuardTest.java | 149 ++++++++++++++++++ .../VersionConsistencyGuardTest.java | 32 ++++ docs/contributing/implementation-guide.md | 75 ++------- docs/templates/v2-layered/README.md | 2 +- .../documentation/DocsBoldFaceGuardTest.java | 146 +++++++++++++++++ 6 files changed, 348 insertions(+), 65 deletions(-) create mode 100644 qa/src/test/java/com/demcha/documentation/DocsBoldFaceGuardTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 94bb520f..98ead4a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,15 @@ follow semantic versioning; release dates are ISO 8601. - **`graph-compose-render-pptx` declares PDFBox.** It compiles against `org.apache.pdfbox` types while declaring only `fontbox`, taking the rest transitively; the resolved version is unchanged. +- **Documentation drift fails the build.** Three checks now cover what four + releases of hand-fixing kept re-breaking: a relative link in the public docs + must resolve, a contributor-facing document must not name a type 2.0 removed, + and a published snippet that names a `*_BOLD` font constant must pair it with a + decoration — the constant resolves to its base family, so without the decoration + the text renders regular. The README release-status block must name a published + version and link the tag it names. `SECURITY.md`, `SUPPORT.md`, `ROADMAP.md` and + `.github/` are scanned for the first time; historical records are skipped by path, + so a new archived page is covered the day it lands. - **The release publishes the showcase it just built.** `cut-release.ps1` never ran `GenerateAllExamples`, so the site was synced from whatever happened to be in `examples/target/generated-pdfs` — nothing at all on a clean checkout, which diff --git a/core/src/test/java/com/demcha/documentation/CanonicalSurfaceGuardTest.java b/core/src/test/java/com/demcha/documentation/CanonicalSurfaceGuardTest.java index cb3b05a4..9a16717f 100644 --- a/core/src/test/java/com/demcha/documentation/CanonicalSurfaceGuardTest.java +++ b/core/src/test/java/com/demcha/documentation/CanonicalSurfaceGuardTest.java @@ -8,6 +8,8 @@ import java.util.List; import java.util.Set; import java.util.TreeSet; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -26,6 +28,42 @@ class CanonicalSurfaceGuardTest { "ModuleYml", "ModuleSummary"); + /** + * Types the 2.0 line removed. Every one is absent from every {@code src/main} + * tree, so a contributor-facing document naming one is teaching code that cannot + * compile — which is how the template-authoring and engine-primitive sections of + * CONTRIBUTING went stale for a full release cycle. + * + *

Deliberately absent: {@code BusinessTheme} survives as an examples-local + * theme helper used by twenty-one example sources, and {@code PptxSemanticBackend} + * still ships beside the fixed-layout PPTX backend. Forbidding either would fail + * the build on text that is correct.

+ */ + private static final List RETIRED_IN_2_0_TOKENS = List.of( + "InvoiceTemplateV2", + "ProposalTemplateV2", + "WeeklyScheduleTemplateV1", + "EntityBounds", + "ParentContainerUpdater", + "CvSpec", + "CvBuilder"); + + /** + * Documents whose job is to record history. A migration guide, an ADR, an archived + * page or the changelog names retired surface on purpose, so the retired-token scan + * skips them by path rather than by a file list — a new historical page is then + * covered the day it is added, instead of failing the build until someone + * remembers to allowlist it. + */ + private static final List HISTORICAL_RECORD_PREFIXES = List.of( + "CHANGELOG.md", + "docs/adr/", + "docs/archive/", + "docs/migration/", + "docs/private/", + "docs/roadmaps/", + "docs/templates/v1-classic/"); + private static final Set MAIN_CANONICAL_SOURCE_ALLOWLIST = Set.of(); private static final Set DOCUMENTATION_ALLOWLIST = Set.of( @@ -117,6 +155,97 @@ void publicMarkdownDocsShouldAvoidLegacySurfaceOutsideHistoricalAuditNotes() thr PUBLIC_MARKDOWN_ALLOWLIST); } + /** + * The contributor-facing surface must not teach types 2.0 removed. + * + *

Scans wider than the legacy check above: {@code SECURITY.md}, + * {@code SUPPORT.md}, {@code ROADMAP.md} and {@code .github/} sit outside every + * existing guard, which is why an issue template could route reporters to a theme + * class that no longer exists and a pull-request template could offer a lane the + * repository dropped.

+ */ + @Test + void contributorFacingDocsShouldNotNameSurfaceRetiredIn2_0() throws IOException { + List roots = List.of( + PROJECT_ROOT.resolve("README.md"), + PROJECT_ROOT.resolve("CONTRIBUTING.md"), + PROJECT_ROOT.resolve("SECURITY.md"), + PROJECT_ROOT.resolve("SUPPORT.md"), + PROJECT_ROOT.resolve("ROADMAP.md"), + PROJECT_ROOT.resolve("examples/README.md"), + PROJECT_ROOT.resolve("docs"), + PROJECT_ROOT.resolve(".github")); + + Set violations = new TreeSet<>(); + for (Path root : roots) { + for (Path doc : markdownUnder(root)) { + String rel = relative(doc); + if (isHistoricalRecord(rel) || PUBLIC_MARKDOWN_ALLOWLIST.contains(rel)) { + continue; + } + String source = Files.readString(doc); + RETIRED_IN_2_0_TOKENS.stream() + .filter(source::contains) + .forEach(token -> violations.add(rel + " names " + token)); + } + } + + assertThat(violations) + .describedAs("these documents name a type 2.0 removed, so anyone following " + + "them writes code that does not compile. A document whose purpose " + + "is to record the removal belongs under one of %s.", + HISTORICAL_RECORD_PREFIXES) + .isEmpty(); + } + + /** + * Every relative link in the public documentation resolves on disk. + * + *

Needs no token list and cannot go stale: it reads what the documents actually + * point at. Renaming a test, archiving a page or deleting an example breaks the + * links to it here rather than for a reader.

+ */ + @Test + void publicMarkdownLinksShouldResolve() throws IOException { + List roots = List.of( + PROJECT_ROOT.resolve("README.md"), + PROJECT_ROOT.resolve("CONTRIBUTING.md"), + PROJECT_ROOT.resolve("SECURITY.md"), + PROJECT_ROOT.resolve("SUPPORT.md"), + PROJECT_ROOT.resolve("ROADMAP.md"), + PROJECT_ROOT.resolve("examples/README.md"), + PROJECT_ROOT.resolve("docs")); + + Pattern link = Pattern.compile("\\]\\(([^)\\s]+)\\)"); + Set broken = new TreeSet<>(); + for (Path root : roots) { + for (Path doc : markdownUnder(root)) { + if (isHistoricalRecord(relative(doc))) { + continue; + } + Matcher matcher = link.matcher(Files.readString(doc)); + while (matcher.find()) { + String target = matcher.group(1); + if (target.startsWith("http") || target.startsWith("mailto:") || target.startsWith("#")) { + continue; + } + String file = target.split("#", 2)[0]; + if (file.isEmpty()) { + continue; + } + if (!Files.exists(doc.getParent().resolve(file).normalize())) { + broken.add(relative(doc) + " -> " + target); + } + } + } + } + + assertThat(broken) + .describedAs("a relative link in the public docs points at a file that does " + + "not exist; the reader gets a 404 on GitHub") + .isEmpty(); + } + @Test void publicAuthoringDocsAndExamplesShouldNotImportEngineInternals() throws IOException { assertNoForbiddenAuthoringImports( @@ -140,6 +269,26 @@ void semanticAuthoringValuePackagesShouldNotImportEngineInternals() throws IOExc PROJECT_ROOT.resolve("core/src/main/java/com/demcha/compose/document/image"))); } + /** Markdown files under a root, or the root itself when it is one. */ + private static List markdownUnder(Path root) throws IOException { + if (Files.isRegularFile(root)) { + return root.toString().endsWith(".md") ? List.of(root) : List.of(); + } + if (!Files.isDirectory(root)) { + return List.of(); + } + try (var paths = Files.walk(root)) { + return paths.filter(Files::isRegularFile) + .filter(path -> path.toString().endsWith(".md")) + .sorted() + .toList(); + } + } + + private static boolean isHistoricalRecord(String relativePath) { + return HISTORICAL_RECORD_PREFIXES.stream().anyMatch(relativePath::startsWith); + } + private void assertNoForbiddenReferences(Path root, Set allowlist) throws IOException { assertNoForbiddenReferences(root, path -> true, allowlist); } diff --git a/core/src/test/java/com/demcha/documentation/VersionConsistencyGuardTest.java b/core/src/test/java/com/demcha/documentation/VersionConsistencyGuardTest.java index a08475d6..52a24242 100644 --- a/core/src/test/java/com/demcha/documentation/VersionConsistencyGuardTest.java +++ b/core/src/test/java/com/demcha/documentation/VersionConsistencyGuardTest.java @@ -216,6 +216,38 @@ void readmeBannerVersionDerivesFromProjectVersion() throws Exception { .isFalse(); } + /** + * The README release-status block advertises a version that exists. + * + *

It used to be a hand-edit no test covered, and the release script demanded it + * already name the version being cut — so between releases {@code develop} carried + * an unpublished version as "latest stable" behind a tag URL that 404s. The script + * rewrites the block now; this pins the result, and pins the link to the text so a + * half-updated block cannot pass.

+ */ + @Test + void readmeReleaseStatusNamesAPublishedVersion() throws Exception { + Set targets = acceptableTargets(); + String readme = Files.readString(PROJECT_ROOT.resolve("README.md")); + + Matcher stable = Pattern.compile( + "\\*\\*Latest stable\\*\\*:\\s*\\[v(\\d+\\.\\d+\\.\\d+)]\\(([^)]+)\\)") + .matcher(readme); + assertThat(stable.find()) + .describedAs("README must carry a '**Latest stable**: [vX.Y.Z](…)' release-status " + + "line — cut-release.ps1 rewrites it during the cut and aborts without it") + .isTrue(); + + assertThat(stable.group(1)) + .describedAs("README 'Latest stable' must name a published release, not the one " + + "in development: its tag page does not exist until the cut (one of %s)", targets) + .isIn(targets); + assertThat(stable.group(2)) + .describedAs("the 'Latest stable' link must point at the tag it names, or the badge " + + "sends readers to a different release than the text claims") + .endsWith("/releases/tag/v" + stable.group(1)); + } + @Test void readmeInstallSnippetsMatchTheProjectVersion() throws Exception { Set targets = acceptableTargets(); diff --git a/docs/contributing/implementation-guide.md b/docs/contributing/implementation-guide.md index e8b3a80e..6f74ab01 100644 --- a/docs/contributing/implementation-guide.md +++ b/docs/contributing/implementation-guide.md @@ -21,34 +21,14 @@ That means a new object usually needs the right answer in four areas: - whether it participates in parent/child layout - how it gets rendered -## Keep `Entity` thin - -`Entity` is the ECS core object, not the preferred home for new layout helpers. - -Use these ownership rules when adding or refactoring engine code: - -- put geometry reads in `EntityBounds` -- put parent container size propagation and page-shift updates in `ParentContainerUpdater` -- keep render-order policy in the render layer (the `engine.render` contracts and the PDF fragment handlers) -- treat `Entity.bounding*` and `Entity.updateParentContainer*` as deprecated compatibility wrappers - -Rule of thumb: - -- if the logic needs `Placement`, `ContentSize`, `Margin`, or parent traversal semantics, it probably belongs in a helper or system utility -- if the logic only needs identity, component access, or canonical child order, it may belong on `Entity` - ## Minimum components a new object usually needs ### Render marker If the object should render something visible, the entity needs a renderable marker component. -Examples: - -- [TextComponent.java](../../core/src/main/java/com/demcha/compose/engine/components/renderable/TextComponent.java) -- [BlockText.java](../../core/src/main/java/com/demcha/compose/engine/components/renderable/BlockText.java) - -Those renderable components are render markers. Prefer keeping them backend-neutral and let renderer-owned handlers perform format-specific drawing. +Keep the marker backend-neutral and let renderer-owned handlers perform the +format-specific drawing. ### Engine markers with different jobs @@ -244,35 +224,31 @@ Preferred extension pattern for new backends: > **Canonical PDF renderer.** Canonical PDF output is produced by > `com.demcha.compose.document.backend.fixed.pdf`: `PdfFixedLayoutBackend` > dispatches each layout fragment to a `PdfFragmentRenderHandler` implementation -> under `document.backend.fixed.pdf.handlers`. The backend-neutral `engine.render` -> *contracts* (`Render`, `RenderPassSession`, `RenderStream`) -> remain the shared render seam — extend PDF drawing by adding or updating a -> fragment handler, not by touching those contracts. +> under `document.backend.fixed.pdf.handlers`. Extend PDF drawing by adding or +> updating a fragment handler, not by widening the backend itself. The PPTX +> backend consumes the same resolved layout through its own handlers, so a new +> fragment kind needs one on each side. Important files: -- [Render.java](../../core/src/main/java/com/demcha/compose/engine/render/Render.java) -- [RenderPassSession.java](../../core/src/main/java/com/demcha/compose/engine/render/RenderPassSession.java) -- [RenderStream.java](../../core/src/main/java/com/demcha/compose/engine/render/RenderStream.java) - [PdfFixedLayoutBackend.java](../../render-pdf/src/main/java/com/demcha/compose/document/backend/fixed/pdf/PdfFixedLayoutBackend.java) - [PdfFragmentRenderHandler.java](../../render-pdf/src/main/java/com/demcha/compose/document/backend/fixed/pdf/PdfFragmentRenderHandler.java) Migration rule for new engine components: -- implement backend-neutral `Render`, not backend-specific render interfaces - move PDF drawing into a `PdfFragmentRenderHandler` under `...document.backend.fixed.pdf.handlers` - use `TextMeasurementSystem` for text width and line metrics instead of reaching through the active renderer - place PDF-only helper objects alongside the backend in `...document.backend.fixed.pdf` -- keep page-surface lifetime in a backend-specific `RenderPassSession`, not in engine builders or render markers - keep resolved draw ordering in the render layer (the PDF fragment handlers), not in pagination utilities - register a render handler for every engine render marker because the PDF entity path no longer supports a backend-specific render fallback -### Render-pass session rules +### Render-pass rules -The current render seam is deliberately narrower than a full backend abstraction. Use these rules when extending it: +The render seam is deliberately narrower than a full backend abstraction. Use +these rules when extending it: -- `RenderStream` should create one render-pass session, not one stream per entity -- renderer orchestrators such as `PdfFixedLayoutBackend` should open one session for the whole pass +- renderer orchestrators such as `PdfFixedLayoutBackend` should open one page + pass for the whole render, not one per entity - single-page handlers should use the session-managed page surface directly - multi-page handlers should request page surfaces explicitly per fragment or page - page creation or annotation-only work should use the session's page-availability helper instead of opening a dummy drawing surface @@ -302,35 +278,6 @@ See [layout-snapshot-testing.md](../operations/layout-snapshot-testing.md) for t So if your new object needs custom drawing, it is not enough to add a builder. You also need a renderable component with the correct renderer implementation. -## Where layout hooks in - -The layout side uses entity components, not builder classes directly. - -Important files: - -- [LayoutTraversalContext.java](../../core/src/main/java/com/demcha/compose/engine/core/LayoutTraversalContext.java) -- [ComputedPosition.java](../../core/src/main/java/com/demcha/compose/engine/components/layout/coordinator/ComputedPosition.java) -- [EntityBounds.java](../../core/src/main/java/com/demcha/compose/engine/components/geometry/EntityBounds.java) -- [ParentContainerUpdater.java](../../core/src/main/java/com/demcha/compose/engine/pagination/ParentContainerUpdater.java) - -In practice: - -- `Anchor`, `Margin`, `Padding`, `ContentSize`, and parent/child links are what matter to layout -- the builder is just the place where you attach those components -- `LayoutTraversalContext` should build one deterministic hierarchy snapshot per pass instead of letting each subsystem rediscover roots and children independently -- `ParentComponent` is the authoritative parent relation, while `Entity.children` is the canonical sibling order -- if those two sources disagree, traversal code should warn loudly and use a deterministic fallback rather than silently hiding the inconsistency -- during pagination, descendants should be resolved before parent containers so parent size updates caused by child page shifts are reflected before parent placement is finalized - -Use the helpers directly when that intent is what you need: - -- read bounds and edges through `EntityBounds` instead of adding more bound helpers to `Entity` -- update parent container size or shifted positions through `ParentContainerUpdater` instead of growing the `Entity` API further - -See [pagination-ordering.md](../architecture/pagination-ordering.md) for a focused explanation of this rule, including why one leaf type can fail while another appears to work. - -If those components are missing or inconsistent, the renderer cannot save you later. - ## Practical checklist for a new object - choose the correct builder base class diff --git a/docs/templates/v2-layered/README.md b/docs/templates/v2-layered/README.md index c1dcc3e8..36a988b5 100644 --- a/docs/templates/v2-layered/README.md +++ b/docs/templates/v2-layered/README.md @@ -6,7 +6,7 @@ > `com.demcha.compose.document.templates.cv`. > > **Naming note:** through 1.x an older surface also called -> "Templates v2" (`CvSpec`, `CvBuilder`, presets with `BusinessTheme`) +> "Templates v2" (the pre-2.0 spec/builder surface, since removed) > shipped alongside this one; it was removed in 2.0 and its docs are > archived at [templates/v1-classic/](../v1-classic/README.md). diff --git a/qa/src/test/java/com/demcha/documentation/DocsBoldFaceGuardTest.java b/qa/src/test/java/com/demcha/documentation/DocsBoldFaceGuardTest.java new file mode 100644 index 00000000..fbc84ba2 --- /dev/null +++ b/qa/src/test/java/com/demcha/documentation/DocsBoldFaceGuardTest.java @@ -0,0 +1,146 @@ +package com.demcha.documentation; + +import com.demcha.compose.qa.RepoPaths; + +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Guards published snippets against the font-face trap. + * + *

{@code FontName.HELVETICA_BOLD} does not select a bold face. {@code FontLibrary} + * resolves it — and every other {@code *_BOLD} / {@code *_ITALIC} / {@code *_OBLIQUE} + * constant — back to its base family, and the face within that family comes from + * {@code DocumentTextStyle.decoration(...)}. A style that names the alias and omits the + * decoration therefore renders regular, which is what six documented headings did.

+ * + *

The failure is silent: the snippet compiles, runs, produces a PDF, and the text is + * simply the wrong weight. Nothing else in the suite looks at it, which is why the + * defect survived in the getting-started guide and four recipes.

+ * + *

Scans published markdown only. Example sources are excluded deliberately: their + * rendered output is pinned by layout snapshots and committed previews, so a weight + * change there is caught by a different gate, and sweeping them would churn every + * committed render.

+ */ +class DocsBoldFaceGuardTest { + + private static final Path PROJECT_ROOT = RepoPaths.repoRoot(); + + /** Documents a reader copies from. */ + private static final List SCANNED = List.of( + "README.md", + "docs", + "core/README.md", + "render-pdf/README.md", + "render-docx/README.md", + "render-pptx/README.md", + "templates/README.md", + "testing/README.md", + "fonts/README.md", + "emoji/README.md"); + + /** + * A face alias: any {@code FontName} constant naming a weight or slant rather than + * a family. Derived from the shape of the name so a new alias is covered on sight. + */ + private static final Pattern FACE_ALIAS = + Pattern.compile("fontName\\(\\s*FontName\\.([A-Z_]*(?:BOLD|ITALIC|OBLIQUE)[A-Z_]*)\\s*\\)"); + + /** Any font selection at all — proves the scan reached live documents. */ + private static final Pattern ANY_FONT_NAME = + Pattern.compile("fontName\\(\\s*FontName\\.[A-Z_]+\\s*\\)"); + + /** Historical records name the old form on purpose. */ + private static final List EXEMPT_PREFIXES = List.of( + "docs/adr/", "docs/archive/", "docs/migration/", "docs/private/", + "docs/roadmaps/", "docs/templates/v1-classic/"); + + @Test + void publishedSnippetsPairAFaceAliasWithADecoration() throws IOException { + Set violations = new TreeSet<>(); + int scannedSites = 0; + + for (Path doc : scannedDocuments()) { + String relative = relative(doc); + if (EXEMPT_PREFIXES.stream().anyMatch(relative::startsWith)) { + continue; + } + String source = Files.readString(doc); + Matcher anyFont = ANY_FONT_NAME.matcher(source); + while (anyFont.find()) { + scannedSites++; + } + Matcher alias = FACE_ALIAS.matcher(source); + while (alias.find()) { + if (!hasDecorationInSameChain(source, alias.end())) { + violations.add(relative + " uses FontName." + alias.group(1) + + " with no decoration(...) — renders regular"); + } + } + } + + assertThat(scannedSites) + .describedAs("found no fontName(FontName.*) site at all: the scan roots moved and " + + "this guard is passing vacuously. Zero *alias* sites is the goal — zero " + + "font selections of any kind means the scan is not reading the docs.") + .isPositive(); + assertThat(violations) + .describedAs("the font name selects the family and the decoration selects the face " + + "within it, so a *_BOLD constant without decoration(...) renders regular. " + + "Name the family and set the decoration.") + .isEmpty(); + } + + /** + * Whether a {@code decoration(...)} call belongs to the same builder chain as the + * {@code fontName(...)} at {@code from}. The chain ends at its {@code build()}; a + * decoration set after that belongs to a different style. + */ + private static boolean hasDecorationInSameChain(String source, int from) { + int chainEnd = source.indexOf(".build()", from); + int windowEnd = chainEnd < 0 ? source.length() : chainEnd; + String chainTail = source.substring(from, windowEnd); + if (chainTail.contains("decoration(")) { + return true; + } + // The decoration may precede the font name in the same chain, so look back to + // where this builder started. + int chainStart = source.lastIndexOf("DocumentTextStyle.builder()", from); + return chainStart >= 0 && source.substring(chainStart, from).contains("decoration("); + } + + private static List scannedDocuments() throws IOException { + List documents = new ArrayList<>(); + for (String entry : SCANNED) { + Path root = PROJECT_ROOT.resolve(entry); + if (Files.isRegularFile(root)) { + documents.add(root); + } else if (Files.isDirectory(root)) { + try (Stream walk = Files.walk(root)) { + walk.filter(Files::isRegularFile) + .filter(path -> path.toString().endsWith(".md")) + .sorted() + .forEach(documents::add); + } + } + } + return documents; + } + + private static String relative(Path path) { + return PROJECT_ROOT.relativize(path).toString().replace('\\', '/'); + } +} From 78d3f4358e0745ffc2cd36cdd2a8aa39524f58e6 Mon Sep 17 00:00:00 2001 From: DemchaAV Date: Fri, 31 Jul 2026 20:05:47 +0100 Subject: [PATCH 2/2] docs: archive the engine guide that still teaches the removed execution layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting the dead names from implementation-guide.md left the model behind. The document still walked a contributor through attaching components to an entity, adding a render marker and a container-growth marker, implementing Breakable, registering through build() and checking entity.hasRender() — forty lines of a pipeline 2.0 removed, and one CONTRIBUTING now contradicts point for point. Breakable, ParentComponent and hasRender are absent from every src/main tree. It is archived rather than rewritten. extension-guide.md already walks the live pipeline end to end — a semantic node, a fluent setter, a render handler, a whole backend, snapshot tests — and package-map.md covers the layout; inventing a replacement for an engine guide risks shipping something plausible and wrong, which is worse than pointing at the documents that are correct. The archived copy opens with what it describes, what removed it, and where to go instead. Breakable, ParentComponent and hasRender( join the retired-token list, so the model cannot return to a live document. Two guards were weaker than the defects they were written for. The retired-token scan could not catch BusinessTheme, because the name survives as an examples-local helper and a repository-wide existence check cannot tell "this type is gone" from "this type is an example's private business". The question that matters to a reader is whether a type ships in a published artifact, so API guidance — issue and pull-request templates, CONTRIBUTING, the template docs — is scanned against its own list. examples/README.md is deliberately outside it: the helper it documents is real. The font guard accepted an alias paired with any decoration, so HELVETICA_BOLD with decoration(ITALIC) passed while rendering neither bold nor what the constant claims. Published snippets now name the family outright; the rule has no exception to get subtly wrong, and the builder-chain analysis it needed is gone. The link check now reads .github too. The retired-token scan already covered it, while the issue templates carry the relative links most likely to break silently. --- CHANGELOG.md | 7 ++ CONTRIBUTING.md | 11 ++- .../CanonicalSurfaceGuardTest.java | 67 ++++++++++++++++++- docs/README.md | 4 +- .../implementation-guide.md | 14 ++++ docs/contributing/extension-guide.md | 5 +- .../documentation/DocsBoldFaceGuardTest.java | 40 ++++------- 7 files changed, 110 insertions(+), 38 deletions(-) rename docs/{contributing => archive}/implementation-guide.md (95%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98ead4a7..33f38777 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,13 @@ follow semantic versioning; release dates are ISO 8601. - **`graph-compose-render-pptx` declares PDFBox.** It compiles against `org.apache.pdfbox` types while declaring only `fontbox`, taking the rest transitively; the resolved version is unchanged. +- **The contributor guide stops teaching a removed architecture.** The engine + implementation guide still walked a new object through attaching components to an + entity, adding a render marker and a container-growth marker, and implementing + `Breakable` — a model 2.0 removed, and one the current contributing guide + contradicts. It is archived, with a banner naming what replaced it; the extension + guide and the package map are now the route for anyone adding a node, a handler or + a backend. - **Documentation drift fails the build.** Three checks now cover what four releases of hand-fixing kept re-breaking: a relative link in the public docs must resolve, a contributor-facing document must not name a type 2.0 removed, diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5839fa84..fd74554f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -11,10 +11,9 @@ Read these files first: - [docs/architecture/package-map.md](./docs/architecture/package-map.md) — what lives in which package, and which of them are internal - [docs/operations/benchmarks.md](./docs/operations/benchmarks.md) when you touch benchmark tooling, render hot paths, layout hot paths, or performance-facing docs -[docs/contributing/implementation-guide.md](./docs/contributing/implementation-guide.md) -goes deeper on the engine, but parts of it still describe the execution layer that -2.0 removed — read `overview.md` and `package-map.md` first and treat the guide as -background until it is rewritten. +[docs/contributing/extension-guide.md](./docs/contributing/extension-guide.md) +walks the extension points end to end — a new semantic node, a fluent setter, a +render handler, a whole backend — and is the guide to follow when adding one. They explain the current public surface, the engine/template split, and the recommended extension points. @@ -320,7 +319,7 @@ For text-heavy primitives, also read: - [TextMeasurementSystem.java](./core/src/main/java/com/demcha/compose/engine/measurement/TextMeasurementSystem.java) - [docs/architecture/overview.md](./docs/architecture/overview.md) -- [docs/contributing/implementation-guide.md](./docs/contributing/implementation-guide.md) +- [docs/contributing/extension-guide.md](./docs/contributing/extension-guide.md) If the primitive should be available to application developers, expose it through `DocumentDsl` and a public `DocumentNode`, not a @@ -364,7 +363,7 @@ If a change affects resolved geometry, pagination, or ordering, prefer adding or - Keep [README.md](./README.md) aligned with the tested examples. - Keep benchmark values clearly dated when they are refreshed. - Keep `assets/readme/*` screenshots consistent with the current render outputs. -- If you add a new extension point or contribution pattern, update [README.md](./README.md), [docs/architecture/overview.md](./docs/architecture/overview.md), and [docs/contributing/implementation-guide.md](./docs/contributing/implementation-guide.md) as part of the same change. +- If you add a new extension point or contribution pattern, update [README.md](./README.md), [docs/architecture/overview.md](./docs/architecture/overview.md), and [docs/contributing/extension-guide.md](./docs/contributing/extension-guide.md) as part of the same change. - If you change benchmark flow, benchmark artifact layout, or diff selection rules, update [README.md](./README.md) and [docs/operations/benchmarks.md](./docs/operations/benchmarks.md) in the same change. - Visual PDF artifacts are grouped under `target/visual-tests/clean/*` and `target/visual-tests/guides/*` so guide-line renders are easy to find separately from clean outputs. diff --git a/core/src/test/java/com/demcha/documentation/CanonicalSurfaceGuardTest.java b/core/src/test/java/com/demcha/documentation/CanonicalSurfaceGuardTest.java index 9a16717f..0ba64a6d 100644 --- a/core/src/test/java/com/demcha/documentation/CanonicalSurfaceGuardTest.java +++ b/core/src/test/java/com/demcha/documentation/CanonicalSurfaceGuardTest.java @@ -45,9 +45,29 @@ class CanonicalSurfaceGuardTest { "WeeklyScheduleTemplateV1", "EntityBounds", "ParentContainerUpdater", + "ParentComponent", + "Breakable", + "hasRender(", "CvSpec", "CvBuilder"); + /** + * Types a contributor must not be pointed at when told how to build against the + * library, even though the name still resolves somewhere in this repository. + * + *

{@code BusinessTheme} is the case that motivated the list: 2.0 removed it from + * the published surface, but the examples keep a local helper of the same name, so a + * repository-wide existence check cannot tell "this type is gone" from "this type is + * an example's private business". The distinction that matters to a reader is + * whether the type ships in a Maven artifact, and for API guidance it does not.

+ * + *

Scanned over API guidance only — issue templates, the contributing guide, the + * template docs — and deliberately not over {@code examples/README.md}, which + * documents that local helper correctly.

+ */ + private static final List FORBIDDEN_IN_API_GUIDANCE = List.of( + "BusinessTheme"); + /** * Documents whose job is to record history. A migration guide, an ADR, an archived * page or the changelog names retired surface on purpose, so the retired-token scan @@ -198,6 +218,47 @@ void contributorFacingDocsShouldNotNameSurfaceRetiredIn2_0() throws IOException .isEmpty(); } + /** + * API guidance must not name a type that no longer ships, even when the name still + * resolves inside this repository. + * + *

Separate from the retired-token scan because the question is different: not + * "does this identifier exist anywhere" but "can a reader of a published artifact + * use it". {@code BusinessTheme} answers yes to the first and no to the second, + * which is exactly how an issue template came to offer it as the theming entry + * point months after 2.0 removed it.

+ */ + @Test + void apiGuidanceShouldNotOfferTypesThatNoLongerShip() throws IOException { + List roots = List.of( + PROJECT_ROOT.resolve("README.md"), + PROJECT_ROOT.resolve("CONTRIBUTING.md"), + PROJECT_ROOT.resolve("docs/templates"), + PROJECT_ROOT.resolve("docs/getting-started.md"), + PROJECT_ROOT.resolve("docs/first-document.md"), + PROJECT_ROOT.resolve(".github")); + + Set violations = new TreeSet<>(); + for (Path root : roots) { + for (Path doc : markdownUnder(root)) { + String rel = relative(doc); + if (isHistoricalRecord(rel) || PUBLIC_MARKDOWN_ALLOWLIST.contains(rel)) { + continue; + } + String source = Files.readString(doc); + FORBIDDEN_IN_API_GUIDANCE.stream() + .filter(source::contains) + .forEach(token -> violations.add(rel + " offers " + token)); + } + } + + assertThat(violations) + .describedAs("these documents tell a reader to build against a type that no " + + "longer ships in any published artifact. A name that survives as an " + + "examples-local helper is still unusable by a consumer.") + .isEmpty(); + } + /** * Every relative link in the public documentation resolves on disk. * @@ -214,7 +275,11 @@ void publicMarkdownLinksShouldResolve() throws IOException { PROJECT_ROOT.resolve("SUPPORT.md"), PROJECT_ROOT.resolve("ROADMAP.md"), PROJECT_ROOT.resolve("examples/README.md"), - PROJECT_ROOT.resolve("docs")); + PROJECT_ROOT.resolve("docs"), + // Issue and pull-request templates carry relative links out of + // .github/ISSUE_TEMPLATE/, two levels deep — the shape most likely + // to break silently when a target moves. + PROJECT_ROOT.resolve(".github")); Pattern link = Pattern.compile("\\]\\(([^)\\s]+)\\)"); Set broken = new TreeSet<>(); diff --git a/docs/README.md b/docs/README.md index 03acd26a..efc6ff57 100644 --- a/docs/README.md +++ b/docs/README.md @@ -19,7 +19,7 @@ back here. | **Designer / author** wanting a custom visual style for CVs | [Templates v2 (layered) — authoring presets](templates/v2-layered/authoring-presets.md) | | **Maintainer of a pre-2.0 caller** (classic `*Spec` + builder templates, removed in 2.0) | [Which template system? — migration map](templates/which-template-system.md) | | **Contributor adding a new template family** to the library | [Templates v2 (layered) — contributor guide](templates/v2-layered/contributor-guide.md) | -| **Contributor extending the engine** (new node type, new backend handler) | [Extension guide](contributing/extension-guide.md) → [Implementation guide](contributing/implementation-guide.md) | +| **Contributor extending the engine** (new node type, new backend handler) | [Extension guide](contributing/extension-guide.md) → [Package map](architecture/package-map.md) | | **Operator** running GraphCompose in production | [Production rendering](operations/production-rendering.md) → [Performance](operations/performance.md) → [Logging](operations/logging.md) | --- @@ -59,7 +59,7 @@ back here. ### Contributing - **[contributing/extension-guide.md](contributing/extension-guide.md)** — add a new node type, backend handler, or theme primitive. -- **[contributing/implementation-guide.md](contributing/implementation-guide.md)** — internal engine notes for contributors hacking on layout / measurement / pagination. +- **[architecture/pagination-ordering.md](architecture/pagination-ordering.md)** — how nodes are paginated and ordered, for contributors working on layout / measurement. - **[contributing/release-process.md](contributing/release-process.md)** — versioning, tag procedure, Maven Central publication. ### Migrations & roadmap diff --git a/docs/contributing/implementation-guide.md b/docs/archive/implementation-guide.md similarity index 95% rename from docs/contributing/implementation-guide.md rename to docs/archive/implementation-guide.md index 6f74ab01..ffcbfbf5 100644 --- a/docs/contributing/implementation-guide.md +++ b/docs/archive/implementation-guide.md @@ -1,3 +1,17 @@ +> **Archived.** This guide describes the Entity-Component-System execution layer +> that 2.0 removed. Its model — attaching components to an entity, render markers, +> a container-growth marker, `Breakable`, `ParentComponent`, `entity.hasRender()` — +> no longer exists in any published module, and following it produces code that does +> not compile. +> +> For the pipeline that ships today — `DocumentNode` → `NodeDefinition` → +> `PreparedNode` → `PlacedFragment` → a fragment handler per fixed-layout backend — +> read [extension-guide.md](../contributing/extension-guide.md) and +> [package-map.md](../architecture/package-map.md). +> +> Kept because the reasoning about pagination and measurement invariants still +> explains why parts of the engine look the way they do. + # Implementation Guide This guide explains how to add new objects and engine extensions in GraphCompose without fighting the current architecture. diff --git a/docs/contributing/extension-guide.md b/docs/contributing/extension-guide.md index baa0d7a6..082e9524 100644 --- a/docs/contributing/extension-guide.md +++ b/docs/contributing/extension-guide.md @@ -236,9 +236,8 @@ Detailed ownership lives in - [`docs/architecture/overview.md`](../architecture/overview.md) — high-level architecture and the canonical-vs-engine boundary. -- [`docs/contributing/implementation-guide.md`](implementation-guide.md) — - engine-side ECS extension patterns (component records, system - registration, low-level harness builders). +- [`docs/architecture/package-map.md`](../architecture/package-map.md) — + what lives in which package, and which of them are internal. - [`docs/architecture/lifecycle.md`](../architecture/lifecycle.md) — the session, layout, and render flow end-to-end. - [ADR 0001 — Shape-as-container](../adr/0001-shape-as-container.md) — diff --git a/qa/src/test/java/com/demcha/documentation/DocsBoldFaceGuardTest.java b/qa/src/test/java/com/demcha/documentation/DocsBoldFaceGuardTest.java index fbc84ba2..20efe499 100644 --- a/qa/src/test/java/com/demcha/documentation/DocsBoldFaceGuardTest.java +++ b/qa/src/test/java/com/demcha/documentation/DocsBoldFaceGuardTest.java @@ -30,6 +30,12 @@ * simply the wrong weight. Nothing else in the suite looks at it, which is why the * defect survived in the getting-started guide and four recipes.

* + *

So published snippets name the family, and the alias is refused outright + * rather than accepted when paired with a decoration. Requiring the pair would still + * admit {@code HELVETICA_BOLD} with {@code decoration(ITALIC)} — not bold, whatever the + * constant says — and would keep publishing a form that reads as the weight set twice. + * A rule with no exceptions is also a rule with nothing to get subtly wrong.

+ * *

Scans published markdown only. Example sources are excluded deliberately: their * rendered output is pinned by layout snapshots and committed previews, so a weight * change there is caught by a different gate, and sweeping them would churn every @@ -69,7 +75,7 @@ class DocsBoldFaceGuardTest { "docs/roadmaps/", "docs/templates/v1-classic/"); @Test - void publishedSnippetsPairAFaceAliasWithADecoration() throws IOException { + void publishedSnippetsNameAFontFamilyRatherThanAFaceAlias() throws IOException { Set violations = new TreeSet<>(); int scannedSites = 0; @@ -85,10 +91,8 @@ void publishedSnippetsPairAFaceAliasWithADecoration() throws IOException { } Matcher alias = FACE_ALIAS.matcher(source); while (alias.find()) { - if (!hasDecorationInSameChain(source, alias.end())) { - violations.add(relative + " uses FontName." + alias.group(1) - + " with no decoration(...) — renders regular"); - } + violations.add(relative + " selects FontName." + alias.group(1) + + " — name the family and set the decoration"); } } @@ -98,30 +102,14 @@ void publishedSnippetsPairAFaceAliasWithADecoration() throws IOException { + "font selections of any kind means the scan is not reading the docs.") .isPositive(); assertThat(violations) - .describedAs("the font name selects the family and the decoration selects the face " - + "within it, so a *_BOLD constant without decoration(...) renders regular. " - + "Name the family and set the decoration.") + .describedAs("a *_BOLD / *_ITALIC / *_OBLIQUE constant is an alias of its base " + + "family, not a face: FontLibrary resolves it back and the face comes " + + "from decoration(...). Pairing the alias with a decoration works but " + + "reads as the weight set twice, and pairing it with a different one " + + "silently contradicts the name. Published snippets name the family.") .isEmpty(); } - /** - * Whether a {@code decoration(...)} call belongs to the same builder chain as the - * {@code fontName(...)} at {@code from}. The chain ends at its {@code build()}; a - * decoration set after that belongs to a different style. - */ - private static boolean hasDecorationInSameChain(String source, int from) { - int chainEnd = source.indexOf(".build()", from); - int windowEnd = chainEnd < 0 ? source.length() : chainEnd; - String chainTail = source.substring(from, windowEnd); - if (chainTail.contains("decoration(")) { - return true; - } - // The decoration may precede the font name in the same chain, so look back to - // where this builder started. - int chainStart = source.lastIndexOf("DocumentTextStyle.builder()", from); - return chainStart >= 0 && source.substring(chainStart, from).contains("decoration("); - } - private static List scannedDocuments() throws IOException { List documents = new ArrayList<>(); for (String entry : SCANNED) {