Repository navigation
fix: refuse incomplete anchor-prepass results - #493
Merged
Merged
Conversation
Codex Review Summary: ✅ CompletedThis 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: fc76440e0c
ℹ️ 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".
The floating-table body-anchor prepass returned a flat Vec<BodyAnchor> whose entries all started at BodyAnchor::default(), so a paragraph the layout never visited was indistinguishable from a legitimate page-top anchor (v_pos 0, starts_page false), and the prepass's own budget diagnostics were discarded. synthesized_body_anchors now returns BodyAnchors: one Option<BodyAnchor> slot per paragraph plus a completion result. A slot stays None when the paragraph was not visited, and completion is false when a page or display-item budget refused a page transition anywhere in the pass, including a floating-table continuation that stopped after every anchor had been recorded. The prepass keeps its private accumulator, so it neither spends the caller's budget nor repeats the document's warnings, and propagates only the diagnostics that invalidate an anchor. synthesize_linesegs publishes anchors only for a complete section. Otherwise that section falls back to the base cursor path it had before floating-table anchors existed and reports the new anchor_prepass_incomplete code once per section. Falling back rather than refusing keeps a hypothetical stop a position degradation instead of a failed render. Regression tests cover a complete pass (including a page-top anchor and its line flag), a stop during paragraph pagination, a stop during a floating-table continuation, and an unvisited later section. Every fixture paginates from font-free flow geometry, and the three incomplete-pass tests fail on the pre-fix consumption path at their default-zero or page-start assertion.
The file is touched by #426, so AGENTS.md requires its development comments in English. This is comment-only: the module doc, the three WMF code docs and the two accumulator method docs, with the same meaning. User-facing Korean strings in this crate are unchanged.
entelecheia
force-pushed
the
fix/anchor-prepass-diagnostics
branch
from
November 10, 2026 14:58
be16dd2 to
6eb5914
Compare
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
layout::synthesized_body_anchorsreturned a flatVec<BodyAnchor>whose entries were allinitialized to
BodyAnchor::default(), and it threw away the private accumulator it ran the layoutwith. A paragraph the layout never reached therefore carried the same value as a genuine page-top
anchor (
v_pos = 0,starts_page = false), andlineseg::synthesize_linesegshad neither acompletion result nor an initialized-entry marker when it consumed those anchors for a floating-table
section.
Change
synthesized_body_anchorsreturnsBodyAnchors: oneOption<BodyAnchor>slot per paragraph plus acompletion result. A slot stays
Nonewhen the paragraph was not visited, and completion is falsewhen a page or display-item budget refused a page transition anywhere in the pass. Both halves are
needed: a floating-table continuation can be refused after every anchor was already recorded, so
the slots alone cannot reveal that stop.
layout_budget_exceeded). The prepass keeps its private accumulator, so it neither spends thecaller's page/display-item budget nor reports the document's other warnings twice - the real layout
pass still reports those once.
RenderIssueCode::AnchorPrepassIncomplete(anchor_prepass_incomplete, severityincomplete,stage
layout), mirrored intorender-report-v1andcertification-report-v1.synthesize_linesegspublishes anchors only for a section whose anchors are complete. Otherwisethat section falls back to the base cursor path it had before floating-table anchors existed and
reports the fallback once per section. A fallback, not a typed refusal: the base cursor is the
pre-existing layout for those sections, so a hypothetical stop degrades positions instead of failing
a render. Policy written up in
docs/design/05-rendering.md§2.2.1 (and the Korean twin).Ordinary inputs are unchanged: the prepass itself sets no page or display-item limit, so neither
completion flag can fire on a document today. The guard exists so the failure boundary is closed
before a limit is ever injected there, which is exactly what the tests do.
Tests
Four regressions in
crates/hwp-render/src/lineseg.rs(anchor_prepass_tests), each paginating fromfont-free flow geometry - a body paragraph advances 1600 HWPUNIT plus 300 of space-after, an empty
cell paragraph is one 1000-HWPUNIT line - with an isolated
FontStore, so nothing depends on a hostface and no assertion counts pages or reads glyph metrics:
complete_prepass_publishes_every_measured_anchor_and_its_page_startv_pos, and the page-top anchor'sstarts_pagereachesLineSeg.flagsbit 0prepass_stop_during_paragraph_pagination_never_publishes_a_default_zero_anchorNone, oneanchor_prepass_incompleteplus the propagatedlayout_budget_exceeded, and the fourth paragraph's synthesizedv_posis not the default zeroprepass_stop_during_floating_table_continuation_still_reports_incompleteSomeyet completion is false, one warning, and the fallback does not apply a page-start flag it did not measureprepass_stop_leaves_a_later_section_unvisited_and_reportedNone, it is still synthesized by the base cursor (nonzerov_pos), and only the floating section reportsRed-first check against the pre-fix behaviour (temporarily restoring the old consumption: anchors
published unconditionally,
slot().unwrap_or_default()):assertion (
anchor_prepass_incompletecount 0 vs 1);(default-zero
v_pos, page-start bit set from an anchor the section was not allowed to use).The complete-pass test passes on both, as it should.
Verification
Hancom open/export verification is a separate person-run evidence leg and was not performed here.
Closes #426.