Repository navigation
fix(cnd): recover text laid out in fully-inline fragment bodies - #1
Merged
Merged
Conversation
Text inside a `block[…]`, a grid cell, a `place` or any user function building on them was silently missing from the CND. A deck whose slides are made of such containers exported only a handful of nodes. Typst does not build a `ParElem` for a fragment body that turns out to be entirely inline: `is_fully_inline_or_neutral` pops the paragraph grouping and reports `FragmentKind::Inline` instead. The text is laid out correctly but never gets a `Location`, so it is invisible to `Introspector::query` — the only source the emit pipeline reads — and is dropped. This also explains why the loss looked arbitrary: a card title survived (its block holds a `v()` and nested blocks, so the grouping finishes) while the card's items did not. `Feature::CndSemantics` forces the paragraph to be built. It is fork-local, excluded from `Feature::all()` and unmapped in the CLI, because it changes layout (an inline fragment becomes `FlowMode::Block`) and must never be on for PDF/HTML output; only typst-cnd's own world enables it. Forcing those paragraphs into existence duplicates text that is already emitted elsewhere, so `emit::ancestry` reconstructs real containment from the laid-out frames, where introspection tags appear as balanced `Tag::Start`/`Tag::End` pairs — the introspector itself returns a flat list. A paragraph is dropped when the nearest enclosing paragraph already carries its text (the body of an inline `box`), when it sits in a container emitted as its own node (list, table, code block, equation, footnote entry), or when every glyph it draws lands inside an `ArtifactElem` (running headers and footers, repeated on every page). Only the nearest enclosing paragraph is consulted: an outer paragraph may repeat the same words by coincidence, and dropping content on that basis would lose it outright. Across the example corpus 18 of 19 fixtures keep an identical node count. The newsletter gains seven genuinely recovered strings and loses two that were pre-existing leaks — a quote's attribution and a figure's caption emitted a second time as paragraphs, both still carried by their own nodes. Its assertion counted those leaks, and now pins the four real prose paragraphs instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A hard line break (`\`) renders as a new line but carries no text of its own, and `extract::walk` emitted nothing for it, so the words on either side were run together in a node's `text`: the title `Plateforme de calcul \ de coût véhicule` exported as "Plateforme de calculde coût véhicule". A node's `text` is a single flat string, and `\` advances the line without ending the paragraph, so the break becomes a space rather than a newline — a newline would suggest a separation Typst does not make. The separator is added during the same walk that computes marker offsets, so citation, footnote and reference spans stay consistent with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`compare_locations` had no caller: `emit` is a private module, so the `pub fn` was unreachable from outside the crate, and the doc comment's "useful for tests" never materialized into a test. Under the CI's `-D warnings` that is a hard error, which failed `cargo test --no-run` on all three platforms and the minimum-Rust-version check. `std::cmp::Ordering` was imported for its return type alone — `sort_by_reading_order` calls `.cmp()` without naming the type — so the import goes with it, or it would just trade one denied warning for another. The crate's clippy and rustfmt failures are untouched here: they predate this branch, are identical on `main`, and span nearly every file of `typst-cnd`. They belong in their own cleanup. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The crate has never satisfied `[workspace.lints.clippy]`, so the `checks` job has failed on every commit since it landed — 57 clippy errors and 103 rustfmt hunks, none of them from the bug fix this branch carries. CI runs with `RUSTFLAGS=-Dwarnings`, which turns each one into a build failure, and that is what hid the rest of the job: the second clippy invocation (`--no-default-features`), `cargo fmt --check` and `cargo doc` had never actually run on this crate. All four pass now. Almost all of it is mechanical — `if` chains collapsed into `&&`, `0usize` separated, `Default::default()` named, redundant annotations dropped. Three places needed a decision: - `is_one` / `is_true` / `is_table_kind` keep their `&T` parameter, with an `allow`: serde's `skip_serializing_if` hands the predicate a reference, so taking the value does not typecheck there. - `list_item` dropped `ordered` and `styles`. They were only ever passed down the recursion: an unordered item carries no number, and its text comes from `extract_text`, which is style-free. - `iter_over_hash_type` is a real question, not noise. The passes that walk `ConvertContext::records` build edges in visit order and then dedup them first-wins, so a hash map's unspecified order was reaching the output. They now go through `records_sorted()`; the two lookups that returned the *first* record matching a label — arbitrary when two nodes share one — go through `find_by_label`, which is that same order. Verified beyond the suite (27 tests, green): the release binary compiles the 40 documents of the `nordwind-tech` corpus to byte-identical node counts and text lengths before and after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`cargo check` and `cargo test` pass locally on code CI rejects, because `.github/workflows/ci.yml` exports `RUSTFLAGS=-Dwarnings` for every job and a local run does not. A dead function or a clippy lint is then a quiet warning here and a build failure there — which is exactly how this branch spent two CI rounds on findings that were reproducible in seconds. The script sets the same environment and runs the gates in CI's order: check, clippy (both feature sets), fmt, doc, the test build, and the typst-cnd suite. Named gates can be run alone: `dev/ci-checks.sh clippy`. It leaves out the jobs that need a toolchain the workflow installs (miri, fuzz) and documents how to run the minimum-Rust-version job, which is the same command on 1.92. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`collapsible_match` folded the recursive arm of `last_paragraph_under_heading` into a guard, but that guard calls `walk`, which writes through `&mut last`. Whether the write happens then depends on arm order — correct as written, silently wrong if the arms are ever reordered. A `let … else` + two `if`s says the same thing with the side effect in the body where it belongs, and satisfies the lint just as well. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Upstream marks the header, the footer and the background as PDF artifacts, but not the foreground. That asymmetry is invisible until `Feature::CndSemantics` turns every fully-inline fragment body into a locatable `ParElem`: the foreground then becomes a paragraph the CND exporter emits once per page. For the usual `foreground: rotate(45deg)[DRAFT]` stamp that is one junk chunk per page in the retrieval index — observed, three copies on a three-page fixture. `ArtifactKind::Watermark` is the kind PDF defines for "text or graphics in the back- or foreground of all pages". Marking it unconditionally would change PDF tagging for every user of the fork, so it is gated on the feature only the CND exporter enables. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit's `records_sorted()` was the wrong tool here: this runs once per labelled element, so sorting every record each time made `rebuild_label_index` quadratic where the code it replaced was linear. `min_by_key` gives the same answer — the smallest id wins rather than whichever the hash map yielded first — in one pass and no allocation. The doc comment also promised more than it delivered, and is corrected here and in `records_sorted` itself: node ids are `Uuid::new_v4()`, so ordering by id is freshly arbitrary on every run. It buys one thing only, that the result does not depend on the map's internal layout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… text Review of the fragment-recovery fix found the comparator in `ancestry.rs` resting on a false premise. It asked whether the enclosing paragraph's text already contains the nested one's. It does not, in general: the nested paragraph is the *realized* box body, while the enclosing paragraph still holds the box unrealized. Anything that only acquires text when realized — a `@ref`'s "Section 1", a citation's marker, a footnote's ordinal, `context` output, a regex show rule — appears in one and not the other, so `#box[see @fig]`, the standard way to keep "Figure 1" unbreakable, was emitted twice. Two signals replace it, each guarding a different mistake: - Nesting in the laid-out frames identifies the box. No text is compared, so a realized marker no longer defeats it. - Source containment keeps a deferred float out. `#place(float: true)` written inside a paragraph is moved to wherever it fits, and its tags land inside a *later* paragraph's still-open range — nesting alone would delete it. Verified rather than assumed: it reproduces on a 170x70pt page. A block-level `place` ends the paragraph it was written in, so the paragraph that ends up enclosing it always begins after it. A `ParElem`'s own span covers only what it starts with (the seven bytes of `Look at` for `Look at #box[…] please.`), so the test needs the union of the spans of everything the paragraph holds — `content_range`. Dropping the nested paragraph would otherwise take edges with it: the enclosing paragraph holds the box body unrealized, so a footnote or citation written inside the box is a bare element there, which the `collect_*` passes ignore by design. `absorb_covered_paragraphs` moves those onto the surviving node, span-less — their offsets index text that node does not expose — and skips any the host already resolved itself, so its own reference is not duplicated. Two more elements join the emitted-container list. A rendered bibliography entry's text belongs to the `bibliography` pool; with the default title the entries are swallowed by the generated heading's source range, but with `title: none` under a hand-written "= References" nothing catches them and each entry leaked as two paragraphs. The outline is emitted as no node at all and every entry repeats a heading the document already carries; it was dropped by accident before, and is now dropped on purpose. `convert.rs` also carries the `records_sorted` comment correction from the previous commit. Verified: 28/28 tests, and the 40 documents of the `nordwind-tech` corpus produce byte-identical node counts and text lengths, so the new rule eats nothing the old one kept. Co-Authored-By: Claude Opus 5 <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 free
to 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.
Le problème
Un deck dont les slides sont construites avec
block,grid,placeet des fonctions utilisateur n'exportait qu'une poignée de nœuds. Sur le document qui a déclenché l'enquête : 9 nœuds au lieu de 86. Titres de section, items de cartes, labels de schémas, corps de texte en colonnes — tout disparaissait silencieusement du.cnd.La cause racine
Typst ne construit pas de
ParElemquand le corps d'un fragment est entièrement inline :is_fully_inline_or_neutralretire le groupement de paragraphe et renvoieFragmentKind::Inline. Le texte est mis en page correctement mais n'obtient jamais deLocation, donc il est invisible àIntrospector::query— la seule source que lit le pipeline d'émission.Vérifié directement :
typst query 'par'sur le document ne renvoyait que les 9 paragraphes déjà émis. Rien n'était jeté par typst-cnd.Ça explique pourquoi la casse semblait arbitraire :
head()→block[inline]card()v()+ blocs → le groupement aboutitcard()punch()line+vdans le bloc placéLe correctif
Feature::CndSemanticsforce la création du paragraphe. Fork-local, exclue deFeature::all()et non mappée dans la CLI : elle change le layout (un fragment inline devientFlowMode::Block) et ne doit jamais être active pour une sortie PDF/HTML. Seul le world de typst-cnd l'active.emit::ancestry(nouveau) corrige les duplications que cette activation provoque. L'introspector renvoie une liste plate ; les frames, eux, portent la hiérarchie réelle sous forme de pairesTag::Start/Tag::Endéquilibrées. Un paragraphe est écarté quand :boxinline) ;ArtifactElem(en-têtes et pieds de page, répétés à chaque page).Seul le paragraphe le plus proche est consulté : un paragraphe plus haut peut répéter les mêmes mots par coïncidence, et écarter sur cette base perdrait le contenu pour de bon.
Le second commit corrige un bug indépendant :
\ne portant aucun texte,extract::walkcollait les mots de part et d'autre (Plateforme de calcul \ de coût→"calculde coût").Vérification
layout_container_content_is_not_losta été confirmé en échec sans le correctif.newslettergagne 7 chaînes réellement récupérées et en perd 2 qui étaient des fuites préexistantes — l'attribution d'une citation et une légende de figure émises une seconde fois en paragraphes, toutes deux toujours portées par leurs nœuds propres. Son assertion comptait ces fuites ; elle épingle désormais les 4 vrais paragraphes de prose.Features::default()et deFeature::all(), la CLI n'a aucun mapping vers elle, etcargo check --workspace --all-targetsest clean.Point de jugement
Faute de spec sur le sujet, un saut de ligne dur devient une espace et non un
\n:\avance la ligne sans terminer le paragraphe, et letextd'un nœud est une chaîne plate — un\nsuggérerait une séparation que Typst ne fait pas. À rediscuter si la spec CND tranche autrement.Ce que la branche contient en plus du correctif
Les trois derniers commits ne touchent pas le comportement, ils rendent la CI verte — elle était rouge sur
maindepuis l'arrivée de la crate, pas à cause de ce correctif :fix(cnd): drop the unused reading-order comparison helper—compare_locationsn'avait aucun appelant ; sous-Dwarningsc'est une erreur dure, et c'est elle qui faisait échouercargo test --no-runsur les trois plateformes et le job « minimum Rust version ».chore(cnd): make typst-cnd pass the workspace clippy lints and rustfmt— 21 fichiers, mécanique : 57 erreurs clippy et 103 hunks rustfmt, tous préexistants. Trois endroits demandaient une décision (prédicats serdeskip_serializing_if, paramètres delist_item, itération surFxHashMap) ; le message du commit les détaille. Vérifié au-delà de la suite : le binaire release compile les 40 documents du corpusnordwind-techvers des comptes de nœuds et longueurs de texte identiques avant/après.chore: add dev/ci-checks.sh— les mêmes gates que.github/workflows/ci.yml, avec sonRUSTFLAGS=-Dwarnings, exécutables en local.Corrections issues de la review
Une review adversariale de la branche a trouvé quatre défauts, tous reproduits avant correction et vérifiés après.
Le comparateur de déduplication reposait sur une prémisse fausse. Il demandait si le texte du paragraphe englobant contient déjà celui du paragraphe imbriqué. Non : l'imbriqué est le corps du
boxréalisé, l'englobant le détient non réalisé. Tout ce qui n'acquiert son texte qu'à la réalisation — « Section 1 » d'un@ref, le marqueur d'une citation, l'ordinal d'une note, la sortie d'uncontext, une règleshow— figure dans l'un et pas dans l'autre.#box[see @fig], l'idiome standard pour garder « Figure 1 » insécable, était émis deux fois. Deux signaux le remplacent : l'imbrication dans les frames (identifie le box) et la containment de portée source (protège un float différé, dont les tags atterrissent dans la portée encore ouverte d'un paragraphe ultérieur — reproduit sur une page 170x70pt). Les arêtes footnote/cite du paragraphe supprimé sont absorbées par l'englobant, sinon elles disparaissaient avec lui.page.foregroundétait émis une fois par page. Upstream marque header, footer et background comme artefacts PDF, mais pas le foreground. Le tamponforeground: rotate(45deg)[DRAFT]produisait un chunk parasite par page. MarquéArtifactKind::Watermark, gaté sur la feature.Une bibliographie
title: nonefuyait en prose. Chaque entrée devenait deux paragraphes en double du pool. Avec le titre par défaut ça ne fuitait que par accident (le skip-range du heading généré les avalait).BibliographyElemetOutlineElemrejoignent la liste des conteneurs émis.find_by_labelretriait toute la map à chaque appel, rendantrebuild_label_indexquadratique. Et le commentaire derecords_sortedpromettait un déterminisme que desUuid::new_v4()ne peuvent pas donner — corrigé dans les deux sens.Nouvelle fixture
inline_boxes_and_floats.typ+ test : compilée avec le binaire d'avant, elle produit les trois paragraphes dupliqués, trois « DRAFT STAMP » et la fuite bibliographique ; après, un seul paragraphe hôte portantrefs+cites+footnotes. Les 40 documents du corpusnordwind-techrestent byte-identiques.Non corrigé, connu : le texte rendu d'un marqueur imbriqué (« Section 1 ») n'apparaît nulle part — le paragraphe hôte garde les mots du box mais pas le marqueur, ce qui était déjà le cas avant la branche. Et l'attribution par corrélation d'index de
ref_edges_from_contentreste imprécise (elle l'était aussi avant).🤖 Generated with Claude Code