feat(ogar-render-askama): T2 — HtmlListView (Redmine spine on ClassView substrate)#83
Merged
Merged
Conversation
…spine on ClassView substrate) Second real emitter (render flavour) per Northstar plan §3. Implements the spec calcified in docs/integration/REDMINE-QUERY-HARVEST.md: lifts Redmine's `app/views/issues/_list.html.erb` shape (17 years of organic evolution) onto our ClassView substrate. Apple meets Apple: Redmine spent years iterating to a substrate-agnostic list partial driven by `Query.inline_columns` + `column_content`. T2 inherits that shape with `RenderColumn` + `ColumnKind` + per-kind cell sub-templates pre-rendered in Rust. ArtifactKind: - Replaces `TsInterface` (deprecated in #81 / anti-pattern #8) with `HtmlListView`. - Existing position in `ALL` preserved (slot 1, append-only contract on the indices held). New surface: - `list_view::RenderColumn` — Redmine's QueryColumn 1:1: name, caption, kind, sortable, groupable, totalable, inline, frozen, default_order. Builder-style setters (`.sortable()` / `.block()` / etc.). - `list_view::ColumnKind` — 10 variants (Plain, IdLink, PrimaryLink, RecordRef, RichText, ProgressBar, RelationList, Hours, AttachmentList, UserList). Append-only; template-stem name pinned by test. - `list_view::default_kind_for(name, type_name)` — resolver picking a sensible default kind from a slot's name + curator type (id→IdLink, primary_label→PrimaryLink, *_ratio→ProgressBar, text+prose→RichText, …). - `artifact_kinds::cells` — one binding-struct per ColumnKind, one askama sub-template each (`dispatch/cell/*.askama`). Each is mass-mail simple per Northstar §1.6. - `artifact_kinds::html_list_view::render_list` — the row-data entry point: takes columns + RowSource stream, pre-renders cells in Rust, feeds the spine template `dispatch/html_list_view.askama`. - `HtmlListViewEmitter` — codebook-only emit (proof-of-shape) for the `for_kind` dispatch path; real callers use `render_list` directly. Two-stage rendering: cells are pre-rendered in Rust (per-kind sub- template) → spine template just emits `{{ cell.body_html|safe }}`. No runtime polymorphism on the askama side; askama's compile-time check applies to every binding individually. Tests (+5 for T2, 20/20 total): - artifact_kind_all_const_enumerates_every_variant updated to expect HtmlListView in slot 1. - html_list_view_proof_of_shape_renders_canonical_concept_header — the codebook emit path surfaces data-class-id + data-concept + empty-state. - html_list_view_renders_inline_and_block_rows — full row stream with IdLink, PrimaryLink, ProgressBar, RichText cells; pins the rendered HTML structure (id="record-42", aria-valuenow="70", block-row class, wiki wrapper, etc.). - html_list_view_renders_group_separator_when_provided — group_header data renders `<tr class="group open">` + name + badge. - default_kind_resolver_is_wired_through_render_kit — pin the public re-export of default_kind_for. - list_view::tests (8 new): RenderColumn builder/defaults, ColumnKind template stems stable, every default_kind_for arm. Workspace check + test green. Per Northstar §1.6, every cell template is the smallest bag of variables it needs; no template hardcodes a class name or a concept-specific slot.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31e51738f8
ℹ️ 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".
| // ── Spine binding struct ───────────────────────────────────────────── | ||
|
|
||
| #[derive(Template)] | ||
| #[template(path = "dispatch/html_list_view.askama", escape = "none")] |
There was a problem hiding this comment.
When render_list is used with data-derived labels (for example a grouped status/custom-field value in GroupHeader::label), escape = "none" disables Askama escaping for the entire spine, so {{ row.group_header_label }}, column captions, titles, and attributes are emitted as raw HTML; the cell bodies already opt into raw output with |safe, so the spine should keep normal HTML escaping and only mark pre-rendered cells safe.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Addressed in #84's branch (which has the rebased list-view + a follow-up commit). Switched escape = "none" → escape = "html" on HtmlListViewCtx; the templates already use |safe only on cell.body_html (pre-rendered), so the substantive change is one attribute. Two-layer escape now: per-cell sub-templates escape their own variables (label/href/…) under their own escape = "html"; the spine receives the already-escaped HTML and passes it through with |safe. Regression test html_list_view_escapes_data_derived_strings_xss_regression pokes every untrusted slot (title, caption, group label, css_classes) with XSS payloads and asserts they all escape to <...> while intentional cell HTML survives. Thanks for catching this.
AdaWorldAPI
pushed a commit
that referenced
this pull request
Jun 20, 2026
…templates Codex P1 on PR #83 (list spine) + PR #84 (detail spine), same root issue: both spine templates were compiled with `escape = "none"`, so EVERY interpolation was raw — a malicious group-header label, title, caption, subtitle, or css_class would inject raw HTML into the page (XSS hazard). Fix: `escape = "html"` on the spine binding-struct attribute. The templates already use `|safe` correctly on the intentionally-pre-rendered HTML fields (`cell.body_html`, `headline_html`, `section.body_html`), so only those bypass escaping. Every other interpolation (title, captions, labels, css_classes, group header labels, subtitle) now gets HTML-escaped by askama. Two-layer escaping for cell content is consistent: - The cell sub-template (e.g. PrimaryLinkCell) escapes its variables (label, href, …) under its own `escape = "html"`. - The spine receives the already-escaped HTML as `body_html` and passes it through with `|safe` (no double-escape). `escape = "none"` retained only on: - `rust_struct.askama` — emits Rust source where `<` etc. must NOT be HTML-escaped. - `cell/rich_text.askama` — wraps trusted pre-rendered prose (Markdown/Textile already expanded upstream). Tests (+2 XSS regressions, 25/25 total): - html_list_view_escapes_data_derived_strings_xss_regression — pokes every untrusted slot (title, caption, group label, css_classes) with XSS payloads (`<script>`, `<img src=x onerror=…>`) + a `<malicious-class>` tag; asserts none survive raw, all escape to `<...>`. Also pins that the PrimaryLink cell's label `<safe>label</safe>` becomes `<safe>label</safe>` (per-cell escape) and that the intentional cell HTML (`<a href="/i/1">`) DOES survive (cell body |safe). - html_detail_view_escapes_data_derived_strings_xss_regression — parallel set: subtitle, inline + block labels, cell + section css. Pins that headline_html (`<a>...</a>`) and rich-text section body (`<p>...</p>`) pass through (marked safe), while every data-derived string is escaped. Workspace check + test green. T3 (HtmlDetailView from #84) folded into this commit since the cherry-picked T2 commit was dropped on rebase against the merged #83 main; this PR delivers both the T3 feature AND the codex P1 fix for the now-merged T2 spine.
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
2 participants
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.
What
T2 per Northstar plan §3 — the second real emitter, render flavour. Implements the spec calcified in
docs/integration/REDMINE-QUERY-HARVEST.md: lifts Redmine'sapp/views/issues/_list.html.erbshape (17 years of organic evolution) onto ourClassViewsubstrate.Apple meets Apple. Redmine spent years finding that one substrate-agnostic list partial driven by
Query.inline_columns + column_contentis the right shape. T2 inherits that shape withRenderColumn+ColumnKind+ per-kind cell sub-templates pre-rendered in Rust.Surface (additive)
list_view::RenderColumnQueryColumn1:1 — name, caption, kind, sortable, groupable, totalable, inline, frozen, default_orderlist_view::ColumnKindlist_view::default_kind_for(name, type_name)artifact_kinds::cells(10 sub-templates)ColumnKindartifact_kinds::html_list_view::render_list(...)RowSourcestream, pre-renders cells, runs spine templateHtmlListViewEmitterfor_kinddispatchHow it renders
Two-stage: cells are pre-rendered in Rust at row-build time via per-kind sub-templates. The spine template (
dispatch/html_list_view.askama) emits{{ cell.body_html|safe }}with zero runtime polymorphism. Askama's compile-time check applies to every binding individually.ArtifactKind change
Replaces
TsInterface(deprecated in #81 / anti-pattern #8) withHtmlListViewat the same slot inArtifactKind::ALL. Existing dispatcher behavior preserved (stub fallback unchanged for the other variants).Tests (20/20)
+5 T2-specific on top of the existing T1/stub set:
html_list_view_proof_of_shape_renders_canonical_concept_header— empty-state path; surfacesdata-class-id="0x0102"+data-concept="project_work_item".html_list_view_renders_inline_and_block_rows— full row stream withIdLink+PrimaryLink+ProgressBar+RichText; pinsid="record-42",aria-valuenow="70",.wikiwrapper,.block-rowclass, rendered prose.html_list_view_renders_group_separator_when_provided— group_header →<tr class="group open">+ name + badge.default_kind_resolver_is_wired_through_render_kit— pins the public re-export contract.+8 list_view module tests: RenderColumn defaults/builder, ColumnKind template-stems stable (the stems ARE filenames — must never change), every
default_kind_forarm.cargo test -p ogar-render-askama20/20; workspace check + workspace test green.What this collapses
_list.html.erbfilesRenderColumnper callcolumn_valuecaseformatter_list.html.erb× controller dirsFollow-ons
T3 (
HtmlDetailView) and T4 (HtmlForm) reuse the sameRenderColumn+ColumnKind+ cell sub-template plumbing — the substantive work was T2. T5 (SurrealqlTable) is independent (codegen flavour).