Skip to content

fix: render list front matter as real properties out of the box (#662)#1338

Merged
chhoumann merged 3 commits into
masterfrom
chhoumann/662-movie-script-properties-regression
Jun 13, 2026
Merged

fix: render list front matter as real properties out of the box (#662)#1338
chhoumann merged 3 commits into
masterfrom
chhoumann/662-movie-script-properties-regression

Conversation

@chhoumann

@chhoumann chhoumann commented Jun 13, 2026

Copy link
Copy Markdown
Owner

Closes #662

Problem

Since Obsidian's properties update, the documented Movie & Series script produced invalid YAML front matter. Multi-value fields (cast, genre, director) came from the script as a YAML-bullet-list-shaped string and were inlined into a quoted scalar, e.g.:

cast: "
  - "[[Ewan McGregor]]"
  - "[[Liam Neeson]]""

…so the property stayed "red" / didn't render. The "Template Property Types" handling that should fix this was gated behind a default-off beta toggle, and its string→list heuristic is effectively dead in the real runtime (metadataTypeManager.getTypeInfo never returns null), so even enabling the toggle didn't help.

Fix

Decouple structured-value collection from the beta flag. Container values (arrays/objects) provided by scripts are now written as real Obsidian List/object properties through the native processFrontMatter serializer — with the toggle off. The toggle now only gates the opt-in string → type heuristic.

  • Narrowed to containers. Numbers/booleans/null are already YAML-safe inlined, so they stay raw — valid front matter (and any comments) stays byte-identical; processFrontMatter only runs when a value genuinely needs it.
  • movies.js: linkifyList returns a real array, so cast/genre/director become List properties of links.
  • Example template rewritten to YAML front matter with quoted scalar wikilinks and the plot in the note body (avoids quote/colon breakage).
  • Latent bug fix: coerce non-string variable values to a string before substitution so the {{VALUE}} scanner doesn't desync (replacement.length).
  • Beta-toggle copy + docs updated to reflect that lists are always handled; the toggle controls string conversion.

Follow-up (also in this PR): capture editor-insertion data-loss

Editor-insertion capture actions (currentLine / newLineAbove / newLineBelow) write to the note body, where front matter property types are meaningless. They formatted inside a collection scope, so after the change above a frontmatter-shaped capture with an array value would be replaced by a [] placeholder that's never applied — silently dropping the value. Collection is now suppressed for these actions, so the value inlines as text (matching pre-change behaviour). File-write capture paths are unchanged.

Verification

  • Dev-vault e2e (toggle OFF): the documented movie template renders cast/genre/director as List-of-link properties; scalars valid; a plot containing " and : is safe in the body; front-matter comments are preserved for scalar-only templates (no needless rewrite). Screenshot-confirmed.
  • Unit: new real-Formatter flag-off regressions (array → List + placeholder, scalars stay raw, same-line coercion), collector getTypeInfo→text cases, and a capture insertion-mode regression on the real run() path. Updated the feature-flag mock tests to match the narrowed behaviour.
  • Full suite: 1899 passed, 0 failed. tsc + eslint clean.

Review

Two design rounds + an adversarial review (3 opposite-model lenses) on the diff. Their findings drove the narrowing-to-containers, the docs/setting-copy correction, the real-Formatter tests, and this capture follow-up.

Known limitations (pre-existing, narrow — not changed here)

  • Append-mode template choices (appendTop/appendBottom) concatenate the template into the note body, so a front-matter block there is body text (arrays inline as plain text). Append can't produce front-matter properties.
  • Unquoted scalar + array in a user's own template: if a raw scalar makes the interim YAML unparseable, processFrontMatter can't apply the collected list (logged warning). The example template + a docs tip show quoting scalar wikilinks.
  • Canvas text capture isn't a property store; containers inline as text there.

Release impact

fix: — patch release. No migration required; existing valid templates are unaffected (byte-identical). Users on the old movies.js should re-download the updated script.

Summary by CodeRabbit

Release Notes

  • Documentation

    • Updated template/property-type docs and the Templates & Properties settings text to clarify that list/object values are always typed, while string-to-typed conversion is a beta option disabled by default.
    • Revised the macro movie/series example to improve YAML front matter usage and move the plot into the note body.
  • Bug Fixes

    • Fixed editor insertion so inline template property values are inserted correctly without leaving empty placeholders.
    • Improved template property collection behavior to prevent unwanted structured-value collection in body-only captures and to ensure consistent typed output for arrays/objects.
    • Improved list wikilink formatting for template/YAML serialization.

… of beta toggle (#662)

The documented Movie & Series script broke since Obsidian's properties update:
multi-value fields (cast/genre/director) were inlined as broken YAML, so the
property showed "red" / failed to render. The "Template Property Types" handling
that fixes this was gated behind a default-off beta flag whose string heuristic
is also effectively dead in the real runtime (metadataTypeManager.getTypeInfo
never returns null), so even enabling it didn't help.

Decouple structured-value collection from the flag: container values
(arrays/objects) from scripts are now always written as proper Obsidian
properties through the native processFrontMatter serializer, even with the
toggle off. The flag now only gates the opt-in string->type heuristic. Scalars
(number/boolean/null) are left raw since they are already YAML-safe inlined,
so valid front matter (and any comments) stays byte-identical.

Also:
- Fix a latent regex.lastIndex desync when a variable value is a non-string
  (coerce the raw replacement to a string).
- movies.js: linkifyList returns a real array so cast/genre/director become
  List properties of links.
- Rewrite the example template to YAML front matter with quoted scalar
  wikilinks and the plot in the note body (avoids quote/colon breakage).
- Update the beta-toggle copy + docs to reflect that lists are always handled
  and the toggle controls string conversion.

Verified flag-off end-to-end in the dev vault (lists render, comments
preserved, plot with quotes/colon safe). Full suite: 1898 passed.
…ures (#662 follow-up)

Editor-insertion capture actions (currentLine / newLineAbove / newLineBelow)
write the capture into the note body at the cursor, but the formatting ran
inside a template-property collection scope. After the #662 change made
container collection flag-independent, a frontmatter-shaped capture with an
array value would be replaced by a "[]" placeholder that is never applied
(applyCapturePropertyVars only runs on the file-write branch) — silently
dropping the value.

Suppress front matter property collection for editor-insertion actions, since
body insertion can't produce a real Obsidian property anyway. The value is
substituted inline as text instead, matching the pre-#662 behaviour and
eliminating the data loss. File-write capture paths (append/prepend/insert-
after/insert-before/top/bottom and create) are unchanged — they correctly pair
collect + apply.

Adds a regression test exercising the real run() path: a newLineBelow capture
of a frontmatter array inlines as text, leaves no placeholder, and never calls
processFrontMatter.
@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4e7096e6-1d8e-449e-be86-34cfdd92b693

📥 Commits

Reviewing files that changed from the base of the PR and between 38a5931 and f3333fd.

📒 Files selected for processing (2)
  • src/engine/CaptureChoiceEngine.template-property-types.test.ts
  • src/engine/CaptureChoiceEngine.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/engine/CaptureChoiceEngine.ts

📝 Walkthrough

Walkthrough

This PR fixes issue #662 (movie script wikilink handling) by decoupling template property collection into two independent control flags: collectionActive (whether to collect) and heuristicEnabled (whether to apply string→structured parsing). The linkifyList script now returns proper arrays for YAML, CaptureChoiceEngine suppresses frontmatter collection during editor insertion, and documentation clarifies the feature's behavior.

Changes

Template Property Types: Fix #662 wikilink handling and decouple collection flags

Layer / File(s) Summary
Core collection logic: two-flag system and container detection
src/utils/yamlValues.ts, src/utils/TemplatePropertyCollector.ts
New isContainerYamlValue helper and CollectArgs contract split featureEnabled into collectionActive (enable/disable collection) and heuristicEnabled (enable/disable string→structured conversion); early-return on inactive collection; restrict non-container scalars when heuristic is disabled.
Movie script: linkifyList returns array for YAML serialization
docs/static/scripts/movies.js
linkifyList(list) now returns string[] of [[...]] wikilinks instead of newline/bullet-formatted strings, fixing frontmatter YAML serialization and resolving issue #662.
Formatter: integrate two-flag collection control
src/formatters/formatter.ts
replaceVariableInString computes collectionActive from template-collection depth and passes both flags to maybeCollect; ensures fallback replacement is explicitly string-coerced.
CaptureChoiceEngine: suppress frontmatter collection for editor insertion
src/engine/CaptureChoiceEngine.ts, src/engine/CaptureChoiceEngine.template-property-types.test.ts
Introduces suppressFrontmatterCollection flag and collectIfFrontmatter helper to prevent orphaned frontmatter placeholders when inserting into editor; applies suppression across both formatting passes in onFileExists and onCreateFileIfItDoesntExist; regression tests verify inline collection and no processFrontMatter call.
Documentation and settings UI: clarify string conversion as beta toggle
docs/docs/Settings.md, docs/docs/TemplatePropertyTypes.md, src/quickAddSettingsTab.ts, docs/docs/Examples/Macro_MovieAndSeriesScript.md
Reframes feature to "Convert string front matter variables to typed properties (Beta)" with examples; clarifies array/object values are always collected via YAML serialization while string→type conversion is optional beta behavior (disabled by default).
Tests: verify two-flag collection semantics and container tracking
src/engine/template-property-types-feature-flag.test.ts
Updated assertions: container values (arrays/objects) collected regardless of flag state; scalar tracking gated behind flag; frontmatter post-processing driven by collected vars presence, not feature flag.
Tests: regression coverage for two-flag collection and beta-toggle-off behavior
src/formatters/formatter-template-property-types.test.ts, src/utils/TemplatePropertyCollector.test.ts
Formatter tests cover beta-toggle-OFF mode verifying container collection and scalar non-collection; collector tests add issue #662 regression with stubbed runtime metadata verifying arrays/objects collected independently of beta flag while string heuristic remains gated by heuristicEnabled.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • chhoumann/quickadd#1154: Updates the template-frontmatter structured value collection/replacement pipeline with YAML-safe placeholders/lists, directly supporting the main PR's collectionActive/heuristicEnabled wiring for frontmatter property handling.
  • chhoumann/quickadd#1204: Refactors CaptureChoiceEngine to delegate frontmatter post-processing to FrontmatterPropertyService, complementing the main PR's suppression of template property collection during editor insertion.
  • chhoumann/quickadd#1017: Adds insertOnNewLineBelow/insertOnNewLineAbove mock utilities to CaptureChoiceEngine tests, which the main PR leverages in regression tests to verify editor insertion paths suppress frontmatter collection.

Suggested labels

released

Poem

🐰 Arrays dance in wikilinks now,
No orphaned brackets left to show,
Two flags guide collection's way—
One turns off, one steers the play!
Issue #662, our rabbit hearts say,
The movies script is fixed today! 🎬

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main fix: rendering list front matter as real properties without requiring the beta toggle.
Linked Issues check ✅ Passed The PR directly addresses issue #662 by fixing multi-value field rendering, updating movies.js to return real arrays, and ensuring proper YAML serialization.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing issue #662: script updates, documentation clarifications, test expansions, and formatter adjustments for property handling.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chhoumann/662-movie-script-properties-regression

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 13, 2026

Copy link
Copy Markdown

Deploying quickadd with  Cloudflare Pages  Cloudflare Pages

Latest commit: f3333fd
Status: ✅  Deploy successful!
Preview URL: https://4566b40e.quickadd.pages.dev
Branch Preview URL: https://chhoumann-662-movie-script-p.quickadd.pages.dev

View logs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 38a5931f05

ℹ️ 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".

Comment on lines +483 to +484
collectionActive,
heuristicEnabled: propertyTypesEnabled && collectionActive,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid collecting body captures as frontmatter

When a Capture choice targets an existing file with append/bottom/insert-after and the capture format starts with frontmatter, onFileExists() runs the first formatting pass inside withTemplatePropertyCollection() before the snippet is placed in the file body. Since this now enables collection for arrays/objects regardless of the beta flag, cast: {{VALUE:cast}} is replaced by cast: []; later applyCapturePropertyVars() only rewrites the target note's frontmatter, so the inserted body snippet is left with the placeholder and the captured values appear in the wrong place. The new suppression only covers currentLine/newLineAbove/newLineBelow, so gate this to cases where the formatted snippet will actually become the file frontmatter.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in f3333fd. The editor-insertion-only suppression was too narrow. Collection is now gated to the single case where the capture content actually becomes the file's own front matter (a brand-new file created from the capture, no template). All existing-file insertions (append/bottom/insert-after/before), template-body embeds, and editor insertions suppress collection and inline the value as text — so no [] placeholder is stranded and nothing is written to the wrong note's front matter. Added a regression test covering append-into-an-existing-file.

…ile's own front matter

Addresses Codex review on #1338: the editor-insertion suppression was too
narrow. A frontmatter-shaped capture appended / inserted-after / written to the
bottom of an EXISTING file also lands in the note body, so collecting it left a
"[]" placeholder in the body while applyCapturePropertyVars wrote the values to
the wrong note's real front matter.

Gate collection to the only case where the capture content actually becomes the
file's front matter: a brand-new file created from the capture with no template.
All existing-file insertions, template-body embeds, and editor insertions now
suppress collection and inline the value as text. Replaces the previous
editor-insertion-only flag.

Adds a regression test for append-into-an-existing-file: the array inlines in
the body, leaves no placeholder, keeps the existing front matter, and never
calls processFrontMatter.
@chhoumann chhoumann merged commit 725b284 into master Jun 13, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Movie script not working anymore since the new properties update.

1 participant