feat(ai): entry transforms (sibling-artifact folding) + flat object-list tables#79
Conversation
…rtifacts
AI export now exposes a per-entry-type `entryTransforms` hook on AIContentConfig.
It runs once per entry and appends returned markdown after the entry's body/fields,
flowing automatically into the per-entry file, collection all.md, and bundles. The
transform receives { contentId, readSibling }, where readSibling is a traversal-
guarded, directory-bound reader (the entry's absolute path is never exposed). Unlike
bodyTransforms, it fires for every format including data-only JSON/YAML entries.
This brings the exporter to parity with the page-render path's colocated-artifact
capability (meta.physicalPath), letting adopters fold a machine-generated sibling
(e.g. <contentId>.profile.json) into AI output natively instead of post-processing
the generated markdown.
…headings The schema-driven serializer rendered an array-of-objects field as deeply nested, one-datum-per-heading markdown (### Label N / #### subfield / value), which is verbose, token-heavy, and hard to scan. Arrays whose object items have only single-line scalar subfields now render as a compact GFM table (columns in schema order, empty cells for absent keys, pipes/newlines escaped). The heading-per-item form is retained for genuinely nested items (subfields that are lists, objects, blocks, or long-form text — e.g. dataset.tables[].columns[]). This is a deterministic, schema-driven decision (no content-length heuristic); fieldTransforms still override the default for any field.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Address branch review + CodeQL alert #19 (js/incomplete-sanitization): - escapeTableCell now escapes backslashes before pipes, so a source value like 'a\\|b' can't leave an unescaped pipe that splits a GFM table row. - makeReadSibling uses fs.lstat instead of fs.stat, so a symlink named as a bare filename inside the entry dir is rejected (isFile() is false) rather than followed out of the directory into published /ai/ output. Adds tests for both and a README note that table cells use default per-type rendering (customize via a fieldTransforms entry on the list field itself).
jpslav
changed the title
Round-2 review follow-ups (all Low/Nit): - formatCellValue renders image subfields as \ in table cells, matching the standalone image rendering instead of falling through to a bare URL string. - Clarify isFlatObjectList JSDoc: it classifies by field type, not by inspecting values. Future-tasks tidy: the sibling-merge and compact-object-list proposals shipped, so their standalone files are removed (rationale now lives in code + ARCHITECTURE/README/CODEBASE_GUIDE). The still-future ideas they raised (first-class sibling-artifact concept; list-form rendering for description-heavy object lists) are migrated into ai-content-v2.md.
debshila
left a comment
debshila
left a comment
There was a problem hiding this comment.
Code review
Solid, well-scoped, well-tested. No blocking issues — a few minor notes below, none of which need to gate merge. Verified against the source rather than the description.
Correctness — verified
- "Computed once, flows to all three outputs" holds by object identity:
processCollectionmutatesaiEntry.appendedSectionsbefore pushing toentries; the per-entry file,all.md(entries.map(e => entryToMarkdown(e, config))), and bundles (allEntries.filter(...).map(...)) all serialize the same object.entryToMarkdownreads the cached field and never re-runs the transform.calledFor).toEqual(['site'])correctly pins this. - Transform runs after the exclusion checks, so excluded entries trigger no IO. Good ordering.
appendedSectionslives onAIEntry, notAIEntryMeta, so it can't leak intomanifest.json.- The backslash-before-pipe escape is genuinely correct:
a\|b→a\\|b→a\\\|b. Thenot.toContain('| a\\\\|b |')assertion is the right regression guard for CodeQL #19.
Security — makeReadSibling is robust
Layered correctly: rejects empty / / / \ / absolute / ..; defense-in-depth path.dirname(abs) !== resolvedDir (also catches .); lstat means a symlink's isFile() is false → rejected, so it can't redirect the read outside the dir. The absolute path is never handed to the transform, so it can't leak into /ai/ output. A null-byte name throws → caught in runEntryTransform → logged and skipped, entry still renders. Good.
Notes (minor, non-blocking)
-
Subfield-level
fieldTransformsare silently dropped in the table path. The old nested-heading branch rendered subfields viarenderField(f, v, ...), which callsapplyFieldTransformkeyed byentryType+ subfield name. The newrenderObjectListTable→formatCellValuenever invokesapplyFieldTransform. So an adopter who hadfieldTransforms[entryType][<subfieldName>]affecting an object-list item now gets default rendering for flat lists. A transform on the list field itself still overrides (verified — checked inrenderFieldbeforerenderListField), but the subfield case isn't called out. Acceptable under "new code, no migrations," but worth a line in the PR description since it's a default-output change. -
Image-cell comment slightly overclaims parity.
formatCellValuerenders(empty alt), while standalonerenderFielduses. Empty alt is reasonable in a table (the column header carries the label) — just tighten the comment to say so rather than "Match the standalone image rendering." -
Entry transforms are awaited sequentially in the per-entry loop. With async
readSiblingIO this serializes across entries. Matches the existing serialawait store.read(...)pattern, so not a regression — flagging only for large-content-set scaling. -
Multi-line
stringsubfields collapse to one cell in tables (classification is by schema type, not value). Already acknowledged and deferred as a future task — good call avoiding a content-length heuristic.
Tests & docs
Test coverage is strong: traversal probes, symlink, throwing transform, once-per-entry caching, MD + JSON, content-ID resolution, missing-sibling baseline-equality, and table variants (empty cells, typed values, pipe/newline/backslash escaping, nested fallback). Docs are thorough and honest — the "appended content is published, don't append secrets/PII" and "static build reads repo checkout vs. runtime reads branch clone" warnings are exactly the gotchas an adopter would hit.
Recommendation: Approve. Optionally address notes 1 and 2; neither blocks merge.
nathanstitt
left a comment
nathanstitt
left a comment
There was a problem hiding this comment.
looks good, left comments in in case they're useful
| return value | ||
| .replace(/\\/g, '\\\\') | ||
| .replace(/\|/g, '\\|') | ||
| .replace(/\r?\n+/g, ' ') |
There was a problem hiding this comment.
Also would probably delete this, I doubt we care about old Mac line endings 😁
Small gap in the newline collapse: /\r?\n+/g matches CRLF and LF, but a lone \r (old-Mac line ending, or a stray \r mid-value) slips through unescaped. A raw CR inside a GFM cell can confuse some renderers. Could fold it into one pass, e.g. .replace(/[\r\n]+/g, ' '), which also collapses mixed \r/\n runs. Edge-casey given cells are schema scalars, but cheap to close.
| ? value.map((v) => resolveSelectLabel(field as SelectFieldConfig, v)).join(', ') | ||
| : resolveSelectLabel(field as SelectFieldConfig, value) | ||
| case 'image': | ||
| // Match the standalone image rendering (renderField) so a table cell stays an image reference. |
There was a problem hiding this comment.
Tiny nit on the comment: the standalone path renders  (alt = label) whereas the cell here is  with empty alt — so it isn't a 1:1 match. Defensible (the column header already carries the label, and alt text in a one-line cell adds little), but "Match the standalone image rendering" slightly overstates it. Maybe "…stays an image reference (alt omitted; the column header labels it)".
| const transform = config?.entryTransforms?.[entry.entryType] | ||
| if (!transform) return | ||
| const ctx: EntryTransformContext = { | ||
| contentId: contentId ?? '', |
There was a problem hiding this comment.
I think this is probably not an issue, I'd probably delete it on a normal review but leaving in case you wanna do something
When extractIdFromFilename returns null, contentId collapses to ''. The README sells the canonical usage as readSibling(`${contentId}.profile.json`) — with an empty id that silently becomes readSibling('.profile.json'), i.e. it probes a dotfile rather than surfacing "this entry has no stable id." In practice every entry filename should carry a Base58 id, so this may be unreachable — if so, an assert/continue reads clearer than a ?? '' that papers over the impossible case. If it is reachable, adopters get a confusing silent miss. Worth a quick think on which it is.
| 'select', | ||
| 'reference', | ||
| 'image', | ||
| ]) |
There was a problem hiding this comment.
this one is all me: I'm probably reading this wrong, but since it's markdown, newlines are breaks, which means a string may not fit into a single line table cell cleanly?
4 participants
Two improvements to the AI content exporter (
packages/canopycms/src/ai/), plus a review-fix pass.1.
entryTransforms— fold in colocated sibling artifactsA new per-entry-type hook on
AIContentConfig:AIEntry.appendedSections, so it flows into the per-entry file, the collectionall.md, and bundles — no post-hoc string surgery, IO runs once.bodyTransforms, MD/MDX-only).readSibling(name)is a traversal-guarded, directory-bound reader (rejects slashes/../absolute,nullon missing, rejects symlinks vialstat). The entry's absolute path is never exposed, so it can't leak into published/ai/output.Brings the exporter to parity with the page-render path's colocated-artifact capability (
meta.physicalPath), so adopters fold a machine-generated sibling (e.g.<contentId>.profile.json) into AI output natively. Per-entry isolation is intentional: a transform sees one entry + its colocated files, not other entries.2. Flat object-list fields render as tables
renderListFieldnow renders arrays of flat-scalar-object items as a compact GFM table instead of nested### Label Nheadings (isFlatObjectListpredicate +renderObjectListTable). Columns in schema order, empty cells for absent keys, pipes/backslashes escaped, newlines collapsed. Genuinely nested items (sub-lists/objects/long-form text, e.g.dataset.tables[].columns[]) keep the heading-per-item form. Deterministic (no content-length heuristic);fieldTransformson the list field still override.3. Review fixes
js/incomplete-sanitization(High, Fix/package normalization #19):escapeTableCellnow escapes backslashes before pipes, soa\|bcan't leave an unescaped pipe that splits a GFM row.makeReadSiblingusesfs.lstatinstead offs.stat, rejecting a symlink named as a bare filename inside the entry dir rather than following it out.Files
ai/types.ts,ai/generate.ts(makeReadSibling,runEntryTransform),ai/json-to-markdown.ts(renderListField,isFlatObjectList,renderObjectListTable,escapeTableCell),ai/index.tsai/__tests__/generate.integration.test.ts+json-to-markdown.test.tsVerification
src/aisuite: 142 passing (entry-transform flows, traversal + symlink guards, table rendering + escaping, once-per-entry caching). Typecheck clean; lint clean; CodeQL Fix/package normalization #19 addressed.