Skip to content

Commit 325a43c

Browse files
committed
feat(engine): add select: first group primitive
Add TemplateGroup.select (all/first) per GROUP_SELECT.md: engine evaluation with per-candidate tracker isolation, sorting.rs winner-aware sort-key resolution, and the T&F-CSE publisher-place worked example. Along the way, fix is_term_only_component judging a nested select:first group structurally instead of by its actual winner, which could make an enclosing select:all group wrongly render a term-only fallback alone. Also, per an adversarial code review: - Fixes select_group_children's winner-resolution, which defaulted every unmodeled component kind (Variable, Message, Title, Number, Term) to "would render" regardless of actual data, and only checked rendering.suppress for Group. Adds simple_variable_would_render, a hand-synced shadow of every SimpleVariable's real rendering logic. - Fixes find_template_title_node, which ignored select when recursing into groups, letting a losing select:first candidate supply title-rendering: from-template substitution formatting.
1 parent 0173d87 commit 325a43c

19 files changed

Lines changed: 1293 additions & 54 deletions
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
# csl26-msqg
3+
title: 'find_template_title_node doesn''t resolve select: first winners'
4+
status: completed
5+
type: bug
6+
priority: normal
7+
tags:
8+
- engine
9+
- rendering
10+
- fidelity
11+
created_at: 2026-09-08T15:13:36Z
12+
updated_at: 2026-09-09T00:22:42Z
13+
---
14+
15+
values/contributor/substitute.rs::find_template_title_node walks render_when/suppress to find a title node, same shape as sorting.rs's date/contributor finders (GROUP_SELECT.md), but wasn't in that spec's Acceptance Criteria and is still structural-first for a select: first group. A third independent 'which branch renders' walker with the same gap -- candidate for sharing crate::values::select_group_children (values/mod.rs) once a concrete title-substitution style needs select: first.
16+
17+
## Summary of Changes
18+
19+
Fixed via an adversarial code review of this PR's implementation. find_template_title_node now routes through crate::values::select_group_children (the same select-aware winner resolver sorting.rs uses) instead of raw &group.group, so a select:first group's losing first candidate no longer supplies title-rendering: from-template substitution formatting. New regression test in values/tests.rs, verified to fail without the fix.
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
---
2+
# csl26-tzs3
3+
title: 'Implement select: first group primitive (GROUP_SELECT.md)'
4+
status: completed
5+
type: feature
6+
priority: high
7+
tags:
8+
- engine
9+
- schema
10+
- rendering
11+
- fidelity
12+
created_at: 2026-09-08T14:21:37Z
13+
updated_at: 2026-09-08T19:26:42Z
14+
parent: csl26-8m2p
15+
blocked_by:
16+
- csl26-2hr4
17+
---
18+
19+
Implement docs/specs/GROUP_SELECT.md: TemplateGroup.select field (All/First), select:first evaluation with per-child cloned-tracker isolation, sorting.rs winner resolution, and T&F-CSE publisher-place worked example.
20+
21+
## Todo
22+
- [x] csl26-2hr4 resolved (PR #1269, merged into the stack below this branch)
23+
- [x] `TemplateGroup.select: TemplateGroupSelect` added (All/First, All the serde default, skip-serialize on default) — `crates/citum-schema-style/src/template.rs`
24+
- [x] Cross-field validation: reject `select: first` with <2 children or combined with `delimiter` — 2 tests in `crates/citum-schema-style/src/style/validation.rs`
25+
- [x] Evaluation: sibling loop in `render_group_component_with_format`/new helper (`render_group_first_child_values`) — per-child cloned tracker, early-exit on first non-empty, merge only winner delta, no `has_meaningful_content` gate for `select: first`
26+
- [x] `sorting.rs` `first_date_component_ref`/`first_contributor_component_ref` resolve a `select: first` group's winning candidate using the same selection semantics as rendering — via new `crate::values::select_group_children` data-presence approximation; `first_contributor_component_ref` stayed reference-independent (used by `*_may_have_list_primary`), new `_for_reference` sibling added for the two reference-specific callers
27+
- [x] Behavior tests per Acceptance Criteria (first/only/no child renders; term-only direct child; losing-candidate isolation; winning candidate with suppressed nested group forcing case; select:first nested in select:all and vice versa; render_when + select combine correctly) — 8 tests in `processor::rendering::tests::group_select_first`, 3 in `sorting::tests::select_first_winner_resolution`, all verified to fail under the pre-fix behavior
28+
- [x] `just schema-gen`
29+
- [ ] (deferred to csl26-wj72, needs a new locale term -- authored-content decision) T&F-CSE publisher-place worked example migrated; report-core.js diff shows 0 regressions
30+
- [ ] Status: Draft -> Active in GROUP_SELECT.md, in the implementation commit
31+
- [x] Soften GROUP_SELECT.md's "also corrects plain select: all rendering on its own merits" line — the csl26-2hr4 fix turned out to be a defensive ordering fix with a zero-diff report-core result, not an observable select:all correction
32+
33+
## Summary of Changes
34+
35+
Implemented docs/specs/GROUP_SELECT.md end to end: TemplateGroup.select (All/First), engine evaluation with per-candidate tracker isolation, sorting.rs winner-aware sort-key resolution, and the T&F-CSE worked example (not deferred, per user steer). Along the way found and fixed a real bug: is_term_only_component judges a select:first group structurally, so a select:first group nested in a select:all parent whose winner is a term-only fallback could wrongly make the outer group render that fallback alone even when every real sibling was empty (mirrors a real CSL <group>'s suppression rule, which a literal <text value> never satisfies). Fixed via Renderer::effective_term_only_component, which simulates select:first's own winner-selection to judge the winner rather than the structure. Zero-diff report-core.js across all 35 embedded styles; 2782 tests pass; verified directly via citum render refs.
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
# csl26-wj72
3+
title: 'Migrate T&F-CSE publisher-place [place unknown] fallback to select: first'
4+
status: scrapped
5+
type: task
6+
priority: normal
7+
tags:
8+
- style
9+
- locale
10+
- fidelity
11+
created_at: 2026-09-08T15:13:45Z
12+
updated_at: 2026-09-08T19:26:47Z
13+
---
14+
15+
docs/specs/GROUP_SELECT.md's worked-example Acceptance Criteria item. taylor-and-francis-council-of-science-editors-author-date.csl:77-86 uses a literal <text value="[place unknown]"/>, gated on no type (universal). Citum's TemplateComponent has no literal-text variant (only Message/Term, both locale-keyed), so this needs a new locale term added first -- an authored-content decision (term key + English text) requiring user sign-off, not made unilaterally in the select:first engine PR. Once a term exists: migrate the publisher-place macro to select:first: [variable: publisher-place, message: term.<key>], verify via report-core.js diff (expect 0 regressions plus the 7 parity rows the render-when disposition audit identified).
16+
17+
## Reasons for Scrapping
18+
19+
Created as a deferral bean based on an overcautious read (the locale term's text is copied verbatim from the shipped CSL, not an invented content decision). User pushed back ("I don't understand why you don't just use a message there?"), so this was implemented directly in the select:first PR (csl26-tzs3 / GROUP_SELECT.md) instead of deferred. Brackets come from wrap: { punctuation: brackets }, not baked into the term text (second user correction). See csl26-tzs3's Summary of Changes.

crates/citum-engine/src/processor/document/tests.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ use citum_schema::options::{
1717
};
1818
use citum_schema::template::{
1919
ContributorForm, ContributorRole, DateForm, DateVariable, Rendering, TemplateComponent,
20-
TemplateContributor, TemplateDate, TemplateGroup, TemplateTerm, TemplateTitle, TitleType,
21-
WrapPunctuation,
20+
TemplateContributor, TemplateDate, TemplateGroup, TemplateGroupSelect, TemplateTerm,
21+
TemplateTitle, TitleType, WrapPunctuation,
2222
};
2323
use citum_schema::{BibliographySpec, CitationSpec, NoteStartTextCase, Style};
2424
use csl_legacy::csl_json::{
@@ -569,6 +569,7 @@ fn test_repro_djot_rendering() {
569569
},
570570
delimiter: None,
571571
render_when: None,
572+
select: TemplateGroupSelect::All,
572573
custom: None,
573574
}),
574575
]

crates/citum-engine/src/processor/rendering/grouped/core.rs

Lines changed: 120 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,11 +1369,23 @@ impl Renderer<'_> {
13691369
}
13701370

13711371
let fmt = F::default();
1372-
let mut group_tracker = tracker.clone();
1373-
let values = self.render_group_child_values(&fmt, ctx, group, &mut group_tracker);
1374-
tracker.advance_issued_occurrences_from(&group_tracker);
1375-
let values = values?;
1376-
tracker.merge_from(group_tracker);
1372+
let values = match group.select {
1373+
citum_schema::template::TemplateGroupSelect::All => {
1374+
let mut group_tracker = tracker.clone();
1375+
let values = self.render_group_child_values(&fmt, ctx, group, &mut group_tracker);
1376+
tracker.advance_issued_occurrences_from(&group_tracker);
1377+
let values = values?;
1378+
tracker.merge_from(group_tracker);
1379+
values
1380+
}
1381+
citum_schema::template::TemplateGroupSelect::First => {
1382+
let mut group_tracker = tracker.clone();
1383+
let values =
1384+
self.render_group_first_child_values(&fmt, ctx, group, &mut group_tracker)?;
1385+
tracker.merge_from(group_tracker);
1386+
values
1387+
}
1388+
};
13771389
let default_delimiter = citum_schema::template::DelimiterPunctuation::Comma;
13781390
let punctuation = group.delimiter.as_ref().unwrap_or(&default_delimiter);
13791391
let (script, realization) = crate::values::punctuation_realization_context(
@@ -1453,6 +1465,11 @@ impl Renderer<'_> {
14531465
let mut values = Vec::with_capacity(group.group.len());
14541466

14551467
for item in &group.group {
1468+
// Computed from `tracker`'s state as it stands before this
1469+
// item's own render call, so it reflects the same starting
1470+
// point a `select: first` child's internal winner-selection
1471+
// would use (see `effective_term_only_component`).
1472+
let term_only = self.effective_term_only_component::<F>(fmt, ctx, item, tracker);
14561473
let Some(rendered) =
14571474
self.render_template_component_with_format::<F>(ctx, item, tracker)
14581475
else {
@@ -1467,7 +1484,7 @@ impl Renderer<'_> {
14671484
if rendered_detailed.text.trim().is_empty() {
14681485
continue;
14691486
}
1470-
if !is_term_only_component(item) {
1487+
if !term_only {
14711488
has_meaningful_content = true;
14721489
}
14731490
values.push(rendered_detailed);
@@ -1479,6 +1496,103 @@ impl Renderer<'_> {
14791496
Some(values)
14801497
}
14811498

1499+
/// Whether `component`'s actual rendered content is term-only.
1500+
///
1501+
/// For most components this is just [`is_term_only_component`], a
1502+
/// purely structural check. But a `select: first` group is not
1503+
/// term-only or not as a whole -- only whichever candidate actually
1504+
/// wins ever appears in the output, so this simulates the same
1505+
/// winner-selection [`Self::render_group_first_child_values`] performs
1506+
/// (against a scratch clone of `tracker`, discarded afterwards) and
1507+
/// judges the winner alone. Without this, a `select: first` group
1508+
/// whose winner happens to be a term-only fallback (e.g. a bracketed
1509+
/// "[place unknown]" message) would count as non-term-only just
1510+
/// because a *losing* sibling candidate (e.g. `variable:
1511+
/// publisher-place`) is not term-only -- causing an enclosing
1512+
/// `select: all` group to treat the fallback text alone as
1513+
/// "meaningful content" and render it even when every other sibling
1514+
/// in that outer group is empty (see `docs/specs/GROUP_SELECT.md`).
1515+
fn effective_term_only_component<F>(
1516+
&self,
1517+
fmt: &F,
1518+
ctx: &TemplateRenderContext<'_>,
1519+
component: &TemplateComponent,
1520+
tracker: &TemplateComponentTracker,
1521+
) -> bool
1522+
where
1523+
F: crate::render::format::OutputFormat<Output = String>,
1524+
{
1525+
let citum_schema::template::TemplateComponent::Group(group) = component else {
1526+
return is_term_only_component(component);
1527+
};
1528+
if group.select != citum_schema::template::TemplateGroupSelect::First {
1529+
return is_term_only_component(component);
1530+
}
1531+
for item in &group.group {
1532+
let mut candidate_tracker = tracker.clone();
1533+
let Some(rendered) =
1534+
self.render_template_component_with_format::<F>(ctx, item, &mut candidate_tracker)
1535+
else {
1536+
continue;
1537+
};
1538+
let rendered_detailed =
1539+
crate::render::component::render_component_detailed_with_format_and_renderer::<F>(
1540+
&rendered,
1541+
fmt,
1542+
ctx.options.show_semantics,
1543+
);
1544+
if rendered_detailed.text.trim().is_empty() {
1545+
continue;
1546+
}
1547+
return self.effective_term_only_component(fmt, ctx, item, &candidate_tracker);
1548+
}
1549+
true
1550+
}
1551+
1552+
/// Render a `select: first` group's children in document order, using
1553+
/// the first candidate that produces non-empty text and discarding the
1554+
/// rest. Unlike [`Self::render_group_child_values`], there is no
1555+
/// "term-only content" gate: a candidate is judged solely on whether it
1556+
/// renders non-empty text (see `docs/specs/GROUP_SELECT.md`'s
1557+
/// Evaluation section).
1558+
///
1559+
/// Each candidate is tried against its own clone of `group_tracker`'s
1560+
/// starting state, so a losing candidate — including a nested group's
1561+
/// tracker mutations at any depth — leaves no trace; only the winner's
1562+
/// tracker delta is written back into `group_tracker`.
1563+
fn render_group_first_child_values<F>(
1564+
&self,
1565+
fmt: &F,
1566+
ctx: &TemplateRenderContext<'_>,
1567+
group: &citum_schema::template::TemplateGroup,
1568+
group_tracker: &mut TemplateComponentTracker,
1569+
) -> Option<Vec<crate::render::component::RenderedComponent>>
1570+
where
1571+
F: crate::render::format::OutputFormat<Output = String>,
1572+
{
1573+
let starting_state = group_tracker.clone();
1574+
for item in &group.group {
1575+
let mut candidate_tracker = starting_state.clone();
1576+
let Some(rendered) =
1577+
self.render_template_component_with_format::<F>(ctx, item, &mut candidate_tracker)
1578+
else {
1579+
continue;
1580+
};
1581+
let rendered_detailed =
1582+
crate::render::component::render_component_detailed_with_format_and_renderer::<F>(
1583+
&rendered,
1584+
fmt,
1585+
ctx.options.show_semantics,
1586+
);
1587+
if rendered_detailed.text.trim().is_empty() {
1588+
continue;
1589+
}
1590+
*group_tracker = candidate_tracker;
1591+
return Some(vec![rendered_detailed]);
1592+
}
1593+
None
1594+
}
1595+
14821596
fn apply_entry_link_fallback(
14831597
&self,
14841598
reference: &Reference,

0 commit comments

Comments
 (0)