Uh oh!
There was an error while loading. Please reload this page.
Gates for the developer guide defects Asciidoctor cannot see - #5669
Open
shai-almog wants to merge 17 commits into
Open
Gates for the developer guide defects Asciidoctor cannot see#5669shai-almog wants to merge 17 commits into
shai-almog wants to merge 17 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:2da41dd8c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
shai-almogforce-pushed
the
devguide-guards
branch
from
September 2, 2026 09:56
2da41dd to
28f950bCompareThere was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:28f950b311
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:21e513112f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Contributor
Cloudflare Preview
|
Contributor
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
shai-almogforce-pushed
the
devguide-guards
branch
from
September 2, 2026 11:39
212956c to
6d0560cCompareThere was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:6d0560cc05
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:b96d1321a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
shai-almogforce-pushed
the
devguide-structure-and-guards
branch
from
September 2, 2026 11:57
6108cad to
e113f04Compareshai-almogforce-pushed
the
devguide-guards
branch
from
September 2, 2026 11:58
b96d132 to
d82c0eaCompareThere was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:d82c0ea525
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
shai-almogforce-pushed
the
devguide-guards
branch
from
September 2, 2026 12:16
d82c0ea to
42189c3CompareThere was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:42189c33d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:89e588ddc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:b88150d821
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Four checks, split out of the structural repairs they were written alongside so each can be reviewed on its own terms. Every one was verified to fail when the defect it describes is reintroduced, and to pass again when it is reverted. check-guide-structure.py -- every document is in the book or declared out of it; no chapter silently becomes a part, a subsection, or a duplicate; and, by rendering the book and reading its headings back, every included chapter's title survives into the output. That last check is what would have caught a whole chapter rendering as subsections of the one before it. check-guide-xrefs.py -- every internal link resolves, and none renders as a bare "[some-id]". Asciidoctor reports neither: a reference carrying link text renders as an ordinary link to nowhere. check-missing-code-blocks.py -- prose that promises a listing where a hole follows. 406 exist, from the snippet extraction in bbdc605, recoverable from bbdc605~1. check-guide-links.py -- links checked against the paths the site actually serves, derived from _redirects and the Hugo content tree and followed through every redirect rule to what it produces, rather than accepted for matching one. The two ratchets may only shrink; growing one needs --allow-new, so recording debt is deliberate rather than a side effect of regenerating. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The stacked content change restored the video capture constraints examples, so their baseline entries went stale and the ratchet refused to pass until they were removed. 406 to 402. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The outcome check compared counts of normalized headings at any depth, which a collision satisfies: "Analytics" is a chapter and also a subsection of Commerce, so the rendered book contains it twice. If the chapter were swallowed, the subsection kept the count at one and the check -- the central safeguard of this change -- passed. Measured on the current book: analytics appears twice at any level and once at chapter level; "getting started" four times at any level and never at chapter level. Manifest entries are now counted against chapter-level headings only. Nested fragments still use the any-level count, because their depth is whatever their parent gives them. Second finding: resolves() tried every matching redirect rule and passed if any chain worked. The host applies the FIRST match and stops, so a link matching an early rule that leads somewhere deleted would have passed on the strength of a later rule the reader never reaches. It now returns the first match's result. No guide link changes verdict today; the point is that the model matches the behaviour it claims to model. Also declined, with the reasoning left in the code rather than a review thread: adding Markdown's three-backtick fence to the literal-block list. The guide contains none, and validate-guide-snippets.py requires every listing to be [source,LANG] with a bare include:: inside ---- delimiters, so such a block would fail that gate before reaching this one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pattern required empty brackets, which is right for ifdef and ifndef -- with content they are the single-line form and guard only that line -- but wrong for ifeval, which has no single-line form and always carries its expression in the brackets. So the directive was listed and never recognised. Latent: the manifest contains no ifeval, and its only conditionals are two single-line ifdef attribute assignments, which are still correctly treated as not opening a block. Fixed because a pattern that names three directives and handles two is a bug in what the code claims, not a missing feature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review asked for AsciiDoc comment-state tracking so that a URL inside a `//` line or a `////` block is skipped. Declining, and recording it where the next reader of this loop will look rather than in a review thread nobody reads. The guide has 120 files, zero commented-out URLs and zero `////` blocks, so the tracking would govern nothing that exists. It would also weaken the gate: with comments skipped, commenting a line out makes its finding disappear and lets the ratchet shrink, banking a "fix" while the dead link stays in the source waiting to be uncommented. Deleting the link is the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review asked for fragment validation on same-site URLs. Measuring first: the guide holds 42 same-site URLs with a fragment, and 38 are /javadoc/ -- the one tree this script cannot enumerate, because it is generated from the framework sources at build time. The other four all pointed at `developer-guide.html#<anchor>`: this book, linking into its own body. Those are the same defect as the fifteen `/manual/*.html` deep links already converted in this stack, missed because they use a different path. They send a reader out to the web to fetch the page they are reading, they do not work in the PDF, and a renamed section breaks them silently -- the anchor lives in the rendered book, so no amount of site-path checking can see it. So the fix is not to validate the fragment, it is to stop writing the link: all four become `<<anchor,text>>` xrefs, which check-guide-xrefs.py already resolves against the rendered anchors. All three targets exist today, so this repairs no live 404 -- it moves four unguarded links under a gate. The rule closes the class rather than the four instances: any codenameone.com link whose path is one of this book's own routes and which carries a fragment is now reported. Verified non-vacuous by restoring one of the four and watching it fail, then reverting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both findings describe constructs the guide does not contain, so neither is a live defect. Both are real holes in the checkers though, and each closes for a few lines, so they close rather than getting argued with. Conditional includes (check-guide-structure.py). A manifest entry inside an ifdef/ifeval was skipped by the duplicate count -- correctly, since the same chapter under two exclusive branches is one chapter in the output -- and also by the rendered-title check, since only one branch renders. Skipping both means a chapter included twice inside a single ACTIVE branch would pass silently. Rather than teach the checker to distinguish exclusive branches for a construct that does not exist (measured: zero include:: lines sit inside a conditional anywhere in the guide), a conditional manifest entry is now refused outright, with a message saying what to extend if one is ever wanted. Trailing slashes on file paths (check-guide-links.py). Site paths were compared with the trailing slash stripped, so a link to `/x.jar/` was validated against `/x.jar`. The reviewer's premise checks out: _redirects declares the two forms as separate routes and spells both out where both work, 32 such pairs. Preserving the slash everywhere would break the many legitimate directory links, so the narrow case is reported instead -- a last path segment containing a dot, wearing a trailing slash. Directory routes such as /blog/ and /javadoc/com/codename1/io/ have no dot in the last segment and are untouched. Both rules verified non-vacuous by introducing an instance and reading the real exit status rather than a pipeline's: structure and links each exit 1 on the probe and 0 once it is reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review is right that the rendered-title check has slack: it counts normalized titles across the whole book, so a nested fragment losing its heading can be covered by an identical heading elsewhere. Confirmed rather than accepted -- removing the leveloffset from both Maven includes takes "Getting started" from 4 rendered headings to 3, while only one file declares it, so `3 >= 1` and the gate passed a genuinely swallowed section. Rather than give nested fragments a rendered identity, which the HTML does not carry, this checks the cause. The spacing rule already existed but ran only on the root manifest, and its condition was "a blank line follows". Both were wrong. Reproduced minimally to find the real rule -- a heading is absorbed only when ALL of these hold: * the include is followed immediately by content, and * neither this include nor the following one carries leveloffset (asciidoctor brackets the content with :leveloffset: attribute entries, and an attribute entry closes the paragraph -- the one on the FOLLOWING include lands between the paragraph and the heading, so it protects just as well), and * the included file does not end on a blank line, and * its last line is ordinary paragraph text -- a delimiter, table row, heading, attribute entry or comment all close the paragraph. That is why _generated-build-hints.adoc, which ends on "|===", does not eat the "Versioned builds" heading directly after it. Against the real book, all four combinations now agree with what asciidoctor actually renders: both leveloffset 4 headings renders ok gate passes only current 4 headings renders ok gate passes only following 4 headings renders ok gate passes neither 3 headings SWALLOWED gate fails An earlier draft exempted only the current include and failed the third row -- a false positive on markup that renders correctly. Every guide include is now checked, not just the manifest's: measured across all 120 files, this leaves zero findings, because the five adjacent includes in Maven-Project-Workflow.asciidoc are protected by leveloffset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the rest
Three link findings, and only the first was a live defect -- in the rule I added
one commit earlier.
The trailing-slash rule was unconditional, so it reported "/videos.html/" as
broken even though _redirects declares that exact source and the site serves it.
The rules here all compile slash-insensitively ("^...$/?"), which is right for
matching but discards the distinction the site actually draws, so the raw sources
are now collected alongside them and a slashed file path is reported only when
_redirects does not spell it out. Probed both ways: "/videos.html/" passes,
"/files/CodenameOneBuildClient.jar/" fails.
Hugo route overrides. hugo.toml sets neither [permalinks] nor uglyURLs today, so
deriving a route as "section path + slug" is currently correct; if either
appears, every route underneath moves and this would go on accepting links to
paths Hugo no longer publishes. Modelling a configuration that is not there would
be guesswork, and reading the built public/ tree makes a local run depend on a
tree that may be stale or absent. So it now notices instead: either key present
aborts the check with what to do about it. Probed with each key in turn.
Protocol-relative links and fragments on ordinary same-site pages are declined,
with the measurements recorded in the code rather than in a review thread nobody
reads. The guide has no `link://` macro at all, and its one bare "//host/path" is
a JavaScript string inside a source block, so widening URL_RE to match "//" would
start reporting code as a broken link. And of the 42 same-site URLs carrying a
fragment, 38 are /javadoc/ -- generated at build time, exempt for that reason --
while the other four pointed into this book and are now xrefs, which
check-guide-xrefs.py resolves against the rendered anchors. Nothing is left that
this script could check without building the Hugo site.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>…ting Two more holes in the guards. Neither has an instance in the guide today, and both close cheaply enough that arguing about them would cost more. check-guide-xrefs.py rendered the book once, with the default backend, so anything inside ifdef::backend-pdf[] was dropped before a single reference in it could be examined -- while the workflow publishes an asciidoctor-pdf build from the same source. Native-Themes.asciidoc already has two such branches. It now renders twice, the second time with -a backend-pdf, which selects exactly the content the PDF build includes without needing asciidoctor-pdf here. The two runs are checked SEPARATELY rather than pooled: an anchor that exists only in the HTML branch must not satisfy a reference made in the PDF branch. Findings name the branch they came from. A/B with a dangling xref planted inside the backend-pdf branch of Native-Themes.asciidoc: old "Cross-references OK: 1741 anchors" -- exit 0 new "Native-Themes.asciidoc:506: <<definitely-not-a-real-anchor>> ... (1 reference(s), pdf render)" -- exit 1 check-missing-code-blocks.py accepted any `[attribute]` line as the start of the promised block, so an admonition standing where a listing used to be hid the hole. An admonition is prose and can never be the listing a sentence promised, so the five names are excluded. Excluded by name rather than by whitelisting the kinds that ARE code: measured, the bracket lines legitimately answering a promising sentence already span [source] (582), [cols=...] and [options=...] (56), [listing] (3), [quote] (2) and an anchored image, and a whitelist would report the next kind nobody anticipated. A/B with each block planted after a promising sentence, using the real signature of a removed listing -- the two blank lines it leaves behind: [NOTE] old: none new new: reported as a hole [source,java] old: none new new: none new The baseline is unchanged at 402, because the guide currently has no admonition sitting in a listing's place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
shai-almogforce-pushed
the
devguide-guards
branch
from
September 2, 2026 13:21
b88150d to
5028cb2Compare…separator Two findings, and this time the first one is live. The rendered book contains exactly three relative hrefs, and all three are broken: css.asciidoc carried link:Images[], link:Fonts[] and link:Supported-Properties#text-decoration[] over from the wiki this guide replaced, naming pages that were never carried across with them. The guide renders as a single page, so a relative href resolves against wherever that page is served and reaches nothing that ships with it -- the reader gets a 404 while every gate reported success. All three targets are sections in that same file: [[Images]] already existed, [[Fonts]] is added to match it, and [[text-decoration]] was already there and already used by a sibling reference. check-guide-xrefs.py now reports any relative href, which after this change has zero instances. Absolute URLs stay with check-guide-links.py, and root-relative paths and fragments are excluded. A/B with one link restored: old printed "Cross-references OK" and exited 0, new reports it and exits 1. The second is a wrong claim in my own comment. check-guide-structure.py returned early when an include was followed by ifdef/ifndef/ifeval/endif, on the reasoning that "a preprocessor directive is not content and cannot absorb a paragraph". True, and irrelevant: asciidoctor REMOVES the directive during preprocessing, so it does not separate anything either. Measured -- with `ifdef::backend-html5[]` between an include ending in prose and a following heading, the render contains zero heading tags and the paragraph reads "...preferred tools. === A parent-local heading". The rule now scans past directive lines to the first line that survives preprocessing. A/B on that construct: old passed it, new reports it. Note the rendered-title check cannot cover this case either, because a parent-local heading is not the first heading of any included file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:f9e50cb8cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
… has a limit The first finding was live: the link ratchet held a working link. docs/website/functions/[[path]].js runs after context.next() has already 404ed and then redirects anything under /files/ or /demos/ to download.codenameone.com. Those paths are served, so the model was missing a whole class of route and had banked a real one -- /files/iOS_UI-Kit.psd -- as broken. The destination is off-site, which puts it in the same bucket as every other off-site redirect: reachable, not verifiable from this repository. The rules are appended AFTER the _redirects rules because the function is a fallback and the first matching rule wins, mirroring how the host evaluates. The baseline shrinks 37 -> 36, and a new /files/ link is now accepted rather than failing CI. The second is a real limit of the PDF surrogate, and one the command line cannot remove. Setting backend-pdf makes ifndef::backend-pdf[] content disappear and ifdef::backend-pdf[] content appear -- verified, and that is the only form this guide uses. It does not undefine backend-html5: the HTML converter sets that itself, after command-line attributes are applied, so even `-a backend-html5!` leaves it defined. Measured both ways. Content guarded on the HTML backend would therefore survive into the surrogate render and could satisfy a PDF-only reference that the real asciidoctor-pdf build leaves dangling. The guide has no such conditional, so rather than model a construct that is not there -- or drive asciidoctor-pdf and try to read anchors back out of a PDF -- the checker refuses one if it appears, naming the alternative the rest of the guide already uses. Probed: planting an ifdef::backend-html5[] block aborts the check with that message. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:6a8963d02a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
…side one
Both scanners matched a delimiter of exactly four characters, so a block opened
with ----- was invisible to them and its contents were scanned as live markup.
The guide has 20 such lines -- eight in Desktop-Integration, six in
Working-With-iOS, six in appendix_goal_generate_archetype -- every one an
ordinary [source] block wrapping an include::. validate-guide-snippets.py already
accepted -{4,}; these two had drifted from it.
Nothing is misread today, because those includes name .java, .xml and .properties
files and the reachability walk skips non-asciidoc targets. The failure mode is
still the interesting one though: it is a FALSE POSITIVE generator, not a missed
defect. A/B with each construct planted inside a five-dash block:
a displayed include::Working-with-UWP.asciidoc[]
old reported a swallowed-heading error against markup that only appears
on the page as text
new clean
a displayed "The wrapper looks like this:" followed by blank lines
old reported it as a missing code block
new clean
Both now track the delimiter that opened the block and close on one of the same
character AND length, which is what asciidoctor does -- so a four-dash line
inside a five-dash block is content rather than the close. The set stays limited
to literal blocks: an example (====) or sidebar (****) block contains live
markup, so headings and includes inside one are real and must keep counting.
Baselines unchanged: 119 included documents, 402 holes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:57212b7f43
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
* Guide figures render with a bundled font instead of the host's
None of the 24 generated figures could be reproduced outside CI. Running the
generator on a Mac reproduces 0 of 24 byte-for-byte, with 3-20% of pixels
differing. Two consecutive local runs are byte-identical, so the generator is
deterministic on one host; the variable is the font.
`PreAdvancedThemingScreenshots` styled with `Font.createSystemFont`, which
resolves through `JavaSEPort.fontFaceSystem` -- "Arial" on macOS and Linux
alike. Arial exists on a developer's Mac and not on a stock CI runner, so AWT
silently substitutes and every glyph changes. The differences are exactly that:
for `flow-layout.png` every differing pixel sits in y 10-130, the title and
label rows, while the colored blocks below match to the pixel.
So the byte-exact gate has only ever been green because CI both generates and
verifies. A developer regenerating locally could not match it, and at the scale
the guide's remaining ~260 app screenshots would need, that is untenable.
The figures now load `native:MainRegular`, which `JavaSEPort.loadTrueTypeFont`
reads from `/com/codename1/impl/javase/Roboto-Medium.ttf` on the classpath
rather than from an installed-font lookup. This is also what the project's font
rule requires everywhere: never `createSystemFont`, always the `native:` scheme.
A null return refuses loudly rather than falling back to a host font, because a
silent fallback would restore the exact non-determinism this removes.
The committed baselines are regenerated from a Mac. CI byte-compares them on
Linux, so the check either passes -- proving host independence rather than
asserting it -- or fails and says so immediately.
Composition, dimensions and colours are unchanged; only the typeface moves.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Pin the font on every component, not just the ones that get block colours
CI reported one stale figure out of 24: `guibuilder-2-insets-3.png`, the only
one containing a `TextField`. The other 23 matched Linux byte for byte, so the
bundled-font change worked -- it just did not reach far enough.
`applyBlockStyleToContent` styled `Label` and `Button`. Everything else kept the
theme's default font, which resolves through the host, so the text field and its
hint were still host-dependent. Enumerating the types that carry text would have
left the next one added broken the same way, so the walk now pins the face on
every component it visits, plus the hint label, which is painted by a `Label`
that is not in the component tree and so is never reached by the walk.
The first attempt used `BLOCK_FONT` for this and regressed the figure: at 29px
against the theme default's 13px the field grew and squeezed "Submit" down to a
clipped sliver. `FIELD_FONT` is sized to reproduce the original height, so the
composition is unchanged and only the typeface moves.
Verified locally by rendering twice, once with `JavaSEPort.setFontFaces` pointed
at a family that does not exist -- which is what a machine without Arial looks
like to the port. All 24 come out byte-identical, so nothing in these figures
reads an installed font any more.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Publish what the runner actually rendered when a screenshot fails
"Committed screenshot is stale: <name>" names the file and nothing else, so
there is no way to tell a real regression from an environment difference
without adding a debugging round trip to CI. The generated directory is now
uploaded as an artifact when the step fails.
Needed immediately: one figure still differs between a Mac and the runner after
the font fix, and host fonts, JDK version and working directory have each been
ruled out locally.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Pin MigLayout's platform, and stop demanding byte equality of a glyph edge
The artifact step paid for itself immediately. Downloading what the runner
rendered showed two figures differing, not the one the gate reported -- it exits
on the first mismatch -- and the two had nothing in common.
`mig-layout.png` differed by 12.8% of its pixels, a real layout shift.
MigLayout takes its default gaps from `PlatformDefaults`, which reads
`System.getProperty("os.name")` and answers MAC_OSX, GNOME or WINDOWS_XP, each
with different spacing. The figure was rendering with macOS gaps on a Mac and
GNOME gaps on the runner. Pinning the platform fixes it: with the pin, 23 of the
24 figures now match the runner's own output byte for byte.
`layered-layout.png` differed by 173 pixels, 0.113%, inside a 25x25 box. That
one is not fixable. Measured against the runner's bytes, the material glyph
lands at exactly the same size and the same origin -- a 55x49 bounding box --
and differs only in antialiased edge coverage, 946 fully-white pixels against
916. Java2D rasterizes the same glyph, from the same bundled font, at the same
size, slightly differently on the two platforms. Demanding byte equality there
would mean deleting legitimate content from the figure or carrying a
permanently red check.
So the comparison moves from `cmp -s` to a comparer that still requires byte
equality by default and accepts a bounded difference only where a figure carries
a `.tolerance` sidecar explaining itself, in the same key=value shape the CN1SS
suites already use. The area bound does the work: a per-pixel delta that large is
meaningless on its own, since a glyph edge flips between white and the block
behind it, but a regression that changed the icon would move far more than 0.3%
of the image.
Verified by running the comparer with the runner's own output against the
committed figures -- which is exactly what CI will do -- and by four probes: an
untoleranced figure that differs fails, the toleranced figure fails when changed
beyond its budget, a missing figure fails the count, and a clean run passes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Guard the iOS font branch, and stop the comparator gating itself out
Two review findings on this PR.
The first says `native:MainRegular` does not give host-independent output,
because `loadTrueTypeFont` resolves native fonts to the first installed SF or
Helvetica family before reaching the bundled Roboto. The branch is real, but it
is reached only when `isIOS` is set, which `loadSkinFile` does for a skin whose
systemFontFamily contains "helvetica" -- and this generator never loads a skin.
The measurement agrees: figures rendered on a Mac match the Linux runner byte
for byte, which could not happen if one side were resolving Helvetica Neue and
the other Roboto.
So the conclusion does not hold today, but the risk is real for tomorrow: a
change that loads a skin here would put host fonts back into the output with no
other symptom. The generator now refuses to run under an iOS platform, and says
why, rather than leaving that to a comment nobody reads.
The second finding is straightforwardly right and is the more serious of the
two. `on.pull_request.paths` triggers on `scripts/developer-guide/**`, but the
`Determine changed components` filter named only two scripts, so a pull request
touching any other script here started the workflow with `docs` false and
skipped the steps that script governs. A change to compare-screenshots.py could
have merged without the screenshot check ever running it -- a gate that skips
itself. The filter now covers the whole directory, which also removes the
two-copies-drift the surrounding comment already warns about.
Verified the guard changes no output: all 24 figures are unchanged, and the
comparer still passes against the runner's own bytes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Count every changed pixel toward the area budget, not just the loud ones
The comparator incremented its mismatch counter only for pixels whose channel
delta EXCEEDED maxChannelDelta, which is what the CN1SS comparator does. With a
sidecar written the way this one was -- a large delta bound paired with a small
area bound -- that leaves an unbounded hole, and review gave the exploit in the
figures' own palette: recolouring the green #06a806 to #a608a6 moves every
channel by exactly 160, so with maxChannelDelta=160 not one pixel is counted and
a dramatically different image reports zero mismatches.
Reproduced it before fixing: the recolour changes 4191 pixels, 2.73% of the
image, at a worst delta of exactly 160 -- and passed.
The two bounds are now independent. maxMismatchPercent limits how much of the
image may change at all, counting every differing pixel; maxChannelDelta caps how
far any single pixel may move. The measured legitimate noise -- 173 pixels,
0.113%, worst delta 141 -- still passes, the recolour now fails on area, and a
five-pixel solid overwrite fails on delta.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Correct the sidecar's rationale to match how the bounds now work
The previous commit changed the comparator so every differing pixel counts
toward the area budget and the channel delta is a separate ceiling. The sidecar
still explained the old behaviour -- that the area bound did the work and the
delta was meaningless on its own -- which is now wrong in a file whose whole
purpose is to justify the numbers beside it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Compare figures in RGBA, because they have an alpha channel
The comparator converted both images to RGB before counting differences. The
committed figures are genuine RGBA PNGs -- four channels, currently opaque
everywhere -- so that dropped a real channel, and any change confined to alpha
was invisible to the tolerance path. A regression that turned the whole figure
transparent while leaving every colour channel intact reported zero changed
pixels and passed.
Byte equality, which every figure without a sidecar is still held to, always
caught this. Only a figure carrying a sidecar could reach the weakened path, so
today the exposure was one image -- but that image is exactly the one whose
comparison is relaxed.
Verified: making layered-layout.png fully transparent with its RGB channels
untouched now reports 100% of pixels changed, the real runner output still
passes, and the recolour and untoleranced-difference probes still fail as they
did.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>…page Two findings, one of them a false positive I introduced last round. Comment blocks. Asciidoctor drops a //// block entirely, so an include inside one does not put its target in the book and prose inside one promises the reader nothing -- but both scanners read straight through. //// joins the fence set in each. A/B with each construct planted inside a comment block: a displayed include:: made the structure check fail, and a promising sentence made the missing-block check fail; both are clean now. Note this is deliberately NOT symmetric with check-guide-links.py, which still reads URLs inside comments. The reasons differ and the code says so: a dead link commented out is still dead debt in the source, and letting the ratchet shrink for it would make "comment the line out" a way to silence that gate. Reachability is the opposite -- calling a commented-out include reachable states something about the book that is simply untrue. Packaged assets. The relative-href rule added last round was too broad: the HTML packaging step copies every subdirectory of docs/developer-guide next to developer-guide.html, so link:img/example.png[] resolves for a reader who opens the zip, and the gate would have rejected it. A relative href that names an existing file under the guide directory is now allowed. sketch/ is not, because that is the one directory the packaging loop skips. Probed all four ways, since an exemption is only worth having if it still rejects: link:img/flow-layout.png[] exit 0 (real, packaged) link:img/does-not-exist.png[] exit 1 link:sketch/whatever.svg[] exit 1 (not packaged) link:Supported-Properties[] exit 1 (the wiki-era shape) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:a6bd5197dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Both findings are defects in the two rules added in the previous commit. packaged_asset() asked whether the file exists here, which is not the question. A link only works if the published output carries it, and the two outputs filter differently: * the HTML archive copies only the SUBDIRECTORIES of docs/developer-guide next to developer-guide.html and skips sketch, so a root-level file is never in the zip however much it exists in the tree; * the website rsync excludes sketch/, *.asciidoc and *.adoc, so a source file nested inside a packaged directory still does not reach the site. So link:Introduction.asciidoc[] was being exempted and readers would follow it to nothing. Both filters are now reproduced. The permissive direction is the expensive one here -- it suppresses a finding for a link nobody can follow -- so all four cases are probed: img/flow-layout.png exit 0 in both outputs Introduction.asciidoc exit 1 in neither img/does-not-exist.png exit 1 sketch/foo.svg exit 1 excluded by both The backend-conditional rejection scanned raw lines, so a source block DISPLAYING `ifdef::backend-html5[]` as example text aborted the whole gate. A displayed directive takes no part in preprocessing. It now tracks literal and comment block delimiters the way the other guide scanners do. A/B: displayed inside a source block went 1 -> 0, while a genuinely active conditional still exits 1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…18 statistics (#5671) * The Introduction taught a dead IDE menu, an iTunes walkthrough and 2018 statistics Six corrections to the chapter a new developer reads first: * The JavaScript target was described twice and contradicted itself: the overview said TeaVM did the translation, the port section said ParparVM with TeaVM as a fallback. The port section is right -- `javascript.port` is declared with `parparvm` as its default and `teavm` documented as "the original builder as a compatibility fallback". * `http://teavm.org:[TeaVM-based builder]` -- the stray colon made the macro a bare host, and the site serves TLS. Removed from the link ratchet, which now stands at 37. * Bitcode was cited as something ParparVM absorbed without modification. Apple has since withdrawn bitcode, which makes the point better than the arrival did, so the sentence now names both moves. * The device-fragmentation section rested on two 2018 Android share numbers, followed by a sentence conceding they would be stale on arrival. The structural claim survives without them. * The iOS developer fee was dated with "for 10 years at the time of this writing". * The UDID instructions walked the reader through iTunes, with a screenshot of iTunes on iOS 9.3.5 that also exposed a real device serial and UDID. iTunes has not existed on macOS since Catalina, and the signing chapter already gives the current answer, so this now points there. * The device build was "a right click away" via the IDE plugin, illustrated by a menu still offering Blackberry, J2ME and Windows Phone builds. Replaced with the `cn1:buildAndroid` and `cn1:buildIos` goals, taken from the mojo names. Both screenshots are deleted, so find_unused_images.py stays satisfied. Verified: asciidoctor at --failure-level WARN, Vale at suggestion level, LanguageTool (status ok, 0 matches, run under JDK 17 rather than the JDK 8 false green), paragraph capitalization, snippets, xrefs, structure, links and missing-code-blocks all clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * "-pl common" made a device build a no-op that reported success Review caught the command I had just written. AbstractBuildWrapperMojo.execute() opens with: if (!project.isExecutionRoot()) { getLog().info("Skipping execution for non-root project"); return; } and BuildAndroidMojo, BuildIosMojo and BuildWin32Mojo all inherit it unchanged. Under `-pl common` the selected module is not the execution root, so the goal logs that line and stops. The design is deliberate -- the wrapper re-invokes Maven on the root pom itself (`request.setPomFile(new File("pom.xml"))`) -- so naming a module both skips the wrapper and defeats its purpose. Measured rather than reasoned, against scripts/hellocodenameone: mvn -pl common ...:buildWin32 -> "Skipping execution for non-root project" BUILD SUCCESS, nothing built mvn ...:buildWin32 -> no skip line, proceeds into the reactor A silent success is the worst failure mode for a getting-started instruction, so the Introduction now says where to run it and warns about the module form explicitly. Working-With-Windows carried the same `-pl common` shape and is fixed with it -- it is where I copied the form from, so leaving it would reintroduce the defect the next time someone follows the pattern. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * The Windows build wrappers named a platform no profile answers to Review checked the command this PR documents and found it still builds nothing. The wrapper passes `codename1.platform` to its nested Maven run, and that property is what activates the module profile in a generated project's root pom. The profile is `win`, matching the value the win module itself declares: <profile><id>win</id> <activation><property> <name>codename1.platform</name><value>win</value> </property></activation> <modules><module>win</module></modules> BuildWin32Mojo and BuildWindowsDeviceMojo both passed "windows", which matches no profile at all. Nothing else in the plugin reads the platform as "windows", so the value was simply inert: the win module never joined the reactor and the nested build reported success having produced no Windows binary. Every other wrapper already agrees with its profile -- android, ios, javascript, linux and javase all match -- so these two were the only ones out of step. Verified by A/B against scripts/hellocodenameone, reading the NESTED reactor rather than the outer one: before hellocodenameone, -common, -javase <- no win module after hellocodenameone, -common, -win The build TARGET is a separate namespace and stays "windows-device" (Executor.BUILD_TARGET_WINDOWS_NATIVE), which is correct. SpotBugs over codenameone-maven-plugin regenerated: 0 findings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * The primary Win32 command had both defects the convenience goal just lost Fixing BuildWin32Mojo made the sentence beside this snippet false: the goal now submits a build, while the command the chapter leads with still could not. It carried both defects at once -- mvn -pl common package -Dcodename1.platform=windows ... -- `-pl common` builds that module instead of the reactor, so the win module is never reached, and `windows` matches no profile even when the reactor is whole. The two mistakes hid each other: with only the module selected, the platform value had nothing left to activate. Corrected to what the wrapper actually runs, read off AbstractBuildWrapperMojo: goal `package`, `codename1.platform=win`, `codename1.buildTarget=windows-device`, from the project root. A/B against scripts/hellocodenameone, reading the reactor, which Maven prints before it compiles: old no win module in the reactor at all new hellocodenameone, -common, -win Both invocations then fail identically on this machine with "invalid target release: 17", because tools/env.sh pins JDK 8 while the demo targets 17. That is environmental and equal on both sides, so it does not affect the comparison. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #5664 so the structural repairs there stop waiting on tooling review. Stacked on it, because these checks are written against a guide that #5664 fixes — run them on master and several fail by design.
Four checks. Each was verified to fail when the defect it describes is reintroduced, and to pass again when it is reverted.
check-guide-structure.pycheck-guide-xrefs.py[some-id]check-missing-code-blocks.pycheck-guide-links.pyHow the link checker decides
It does not carry a list of good links. It derives what the site serves and follows the rules to what they produce:
_redirectssources compiled to patterns, splat and:placeholderincluded, then followed to their destination rather than accepted for matching./*.htmlmatches every root-level.htmlpath there is, so a match proves nothing.url,slug,aliases— because 1055 of the 1262 content pages override their route withslug, and deriving it from the filename both invents routes and rejects real ones.buildDraftsandbuildFutureto false.index.htmlis not a route./developer-guide/derived from whatscripts/website/build.shrsyncs. Only/javadoc/stays exempt, because it is produced from the framework sources and cannot be enumerated here.Ratchets
check-missing-code-blocks.pyrecords 406 existing holes andcheck-guide-links.py38 bad links. Both are multisets — one line per occurrence, because a file naming the same bad URL twice has two problems.Both may only shrink. A fixed entry that is still baselined fails, so the ratchet actually banks its fixes; growing one requires
--allow-new, so recording new debt is deliberate rather than a side effect of regenerating.Where they run
developer-guide-docs.ymlruns all four.check-guide-links.pyalso runs fromwebsite-docs.yml, which triggers ondocs/website/**— a page renamed or re-slugged there breaks the guide, and putting those paths into the guide workflow would run the whole guide build, maven install and screenshot generation included, for every blog post the daily publisher pushes.The
docspaths filter now coversscripts/developer-guide/**rather than two named files, because a change to a script not in that filter started the workflow while skipping the steps that script governs.