feat(notes): ship a Bases-friendly default episode note template (#160)#201
Open
chhoumann wants to merge 5 commits into
Open
feat(notes): ship a Bases-friendly default episode note template (#160)#201chhoumann wants to merge 5 commits into
chhoumann wants to merge 5 commits into
Conversation
Deploying podnotes with
|
| Latest commit: |
b698643
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3458bb71.podnotes.pages.dev |
| Branch Preview URL: | https://chhoumann-160-bases-default.podnotes.pages.dev |
![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: c695f8444a
ℹ️ 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".
The episode note path/template both defaulted to "" (empty), which left the
"Create episode note" command disabled on a fresh install and gave new users
no structured metadata. Ship sensible defaults:
- note.path: PodNotes/{{podcast}}/{{title}}.md (matches the download convention)
- note.template: Bases-friendly frontmatter (type/podcast-link/url/date/tags +
user-fillable status/rating/favorite), with the raw title as the body H1 so
YAML rules never apply to it. {{podcastlink}} ties each episode to its #163
feed note for Bases/Dataview rollups.
Make the frontmatter bulletproof: NoteTemplateEngine now sanitizes the URL tags
({{url}}/{{stream}}/{{artwork}}/{{episodeurl}}/{{episodeartwork}}/{{feedurl}}/
{{feedartwork}}) by stripping " and \\ (lossless for valid URLs), mirroring
FeedNoteTemplateEngine, so a quoted scalar like url: "{{url}}" stays valid.
Migrate existing users without clobbering customizations: migrateNotePath /
migrateNoteTemplate upgrade ONLY the exact legacy empty value (or absent), so
any non-empty path/template the user configured is preserved. Mirrors the #183
migrateDownloadPath precedent and is wired into loadSettings.
Update the e2e provisioning seed to match (keeps the seed/DEFAULT_SETTINGS drift
test green and seeds the new default for verification), refresh the settings
placeholder, and document the default + an example .base view in templates.md.
Verified in the isolated worktree Obsidian vault: notes created via the real
create-podcast-note command (incl. a hostile title/URL and a no-date episode)
parse into clean frontmatter via Obsidian's properties parser.
…review) Address ultracode review findings on the #160 default template: - NoteTemplateEngine: guard {{url}}/{{stream}}/{{episodeurl}} with `?? ""` before sanitizeUrlForTemplate, matching the other URL tags. A corrupted/hand-edited data.json could surface a null/undefined on these typed-string fields, where the unguarded `.replace` would throw and abort note creation. Add a regression test covering a corrupted episode. - docs/templates.md: the default puts {{artwork}} in the body as a Markdown image, not a quoted frontmatter scalar; correct the YAML-safety bullet to say so. - docs/templates.md: quote the example .base filter condition ('type == "podcastEpisode"') to match Obsidian's canonical Bases syntax.
…eview) Address findings from the adversarial review of the #160 default template: - Migration: only upgrade the legacy empty note when the WHOLE note is the old default (path AND template both empty/absent). Replace the per-field migrateNotePath/migrateNoteTemplate with migrateNoteSettings so a user who configured one field and deliberately left the other empty (e.g. a custom path with an empty template to keep "Create episode note" disabled) is never silently re-enabled. Coalesce null fields so a corrupted data.json can't reach the path/template engines. Update tests to cover the preserve-partial case. - docs/templates.md: the example .base used a `sort:` view key, which is not part of the documented Obsidian Bases schema (sorting is view UI state; only `order` + `groupBy` are documented). Switch to `groupBy` (by listening status) and `note.`-prefixed properties, matching the canonical syntax. - Qualify the "always valid YAML" claim (docs + constants comment): the guarantee holds for the default and any ordinary feed-note path; a feed-note path with a literal quote/backslash in a folder segment would flow into {{podcastlink}}.
…review)
Codex review (P2): globally stripping "/\ from NoteTemplateEngine's URL tags
corrupts {{url}} for local-file episodes, where episode.url is a wikilink from
generateMarkdownLink (e.g. a file named Talk "A".mp3 would link to Talk A.mp3),
and changes existing custom templates that use {{url}} outside YAML.
Revert the URL-tag sanitization added earlier in this branch back to master's
verbatim behavior. The default frontmatter stays valid YAML for the common case
without mutating tag values: feed episode URLs are well-formed (no quotes), the
{{podcastlink}} name is already sanitized, and ordinary local-file links contain
no quotes. A URL or file name with a literal quote is the one case that would
need adjusting, which is documented. sanitizeUrlForTemplate stays in use by
FeedNoteTemplateEngine, where {{url}} is always a real website URL.
Add a test guarding that {{url}}/{{episodeurl}} pass through verbatim (local-file
wikilinks preserved), and update the docs/comment claims accordingly.
chhoumann
force-pushed
the
chhoumann/160-bases-default-template
branch
from
c695f84 to
1d59e3c
Compare
Owner
Author
|
@codex review Rebased onto master (resolved the src/constants.ts overlap with #114, keeping both the new episodeListLimit setting and the note.template default) and addressed your P2 by reverting the URL-tag sanitization to verbatim rendering in NoteTemplateEngine (commit 1d59e3c). Gates are green. Could you take another look? |
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d59e3c41a
ℹ️ 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".
…review)
Codex review (P2): after rendering URL tags verbatim, a local-file episode whose
name contains a double quote (e.g. Talk "A".mp3, playable on macOS/Linux) makes
{{url}} a wikilink with a quote, so url: "{{url}}" renders
url: "[[Talk "A".mp3]]" and the whole frontmatter fails to parse, hiding every
property from Bases.
Move {{url}} (and it was already true for {{artwork}}) into the note body, where
YAML rules don't apply, so the verbatim tag value can never break the
frontmatter. The frontmatter now contains only values that can't carry a
YAML-hostile character (type, the sanitized {{podcastlink}}, an ISO/empty date,
tags, and the user-fillable status/rating/favorite). Feed-only users can add
url: "{{url}}" back; documented. Update the example, the e2e seed, and the test
(now exercises a quote-bearing local-file url to prove the frontmatter stays
parseable).
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
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.
Summary
Closes #160.
The episode note defaults shipped empty (
note.path = "",note.template = ""), which left the "Create episode note" command disabled on a fresh install and gave new users no structured metadata to build Obsidian Bases views on. This PR ships a sensible, Bases-friendly default for both:note.path:PodNotes/{{podcast}}/{{title}}.md(matches the existingdownload.pathconvention).note.template: YAML frontmatter with structured properties Bases can sort, filter, and group on, plus user-fillable tracking fields:{{podcastlink}}ties each episode to its podcast feed note (the #163 feature) so Bases / Dataview can roll episodes up under their show. The raw{{title}}lives in the body H1, where YAML rules do not apply, so awkward titles never break the frontmatter.What changed
src/constants.ts- newnote.path/note.templatedefaults (scoped to thenoteblock only).src/TemplateEngine.ts-NoteTemplateEnginenow sanitizes the URL tags (url,stream,artwork,episodeurl,episodeartwork,feedurl,feedartwork) with the existingsanitizeUrlForTemplate(strips"and\), mirroringFeedNoteTemplateEngine. This is lossless for well-formed URLs and keeps a quoted scalar likeurl: "{{url}}"valid. Each tag is guarded with?? ""so a corrupteddata.jsoncan't throw.src/settingsMigrations.ts- newmigrateNoteSettings, which upgrades the legacy empty note to the new default only when the whole note is the old default (both path and template empty/absent). Any configured field is preserved verbatim, including a deliberately-empty field used to keep note creation disabled.src/main.ts-loadSettingsappliesmigrateNoteSettings.scripts/provision-obsidian-e2e-vault.mjs- the e2e seed note block now matchesDEFAULT_SETTINGS(keeps the seed/DEFAULT_SETTINGS drift test green and seeds the new default for verification).src/ui/settings/PodNotesSettingsTab.ts- the note-template placeholder now shows a Bases-friendly frontmatter hint instead of the old Dataview::style.docs/docs/templates.md- documents the new default, an example.baseview, and the URL-tag sanitization.Compatibility / migration impact
Defaults only reach new installs and users who never configured note settings. Existing users are protected:
{ path: "", template: "" }). A user who customized either field keeps both fields untouched.migrateDownloadPath).The URL-tag sanitization changes existing tag output only for malformed inputs (a
"or\in a URL, which valid URLs never contain). For local-file episodes{{url}}is a wikilink; a normal filename is unaffected, and a filename containing a literal"would have its quote stripped (rare; valid YAML is preferred over an exact link in that edge case).Verification
Gates (Node 22), all green:
Real-Obsidian verification in an isolated worktree vault (
npm run obsidian:e2e): drove the actualcreate-podcast-notecommand for several episodes, including a hostile title/URL (Why "AI": a deep dive: part 2, URL with an embedded quote and backslash) and a no-date episode. Obsidian's properties parser produced clean frontmatter in every case:url: "https://example.com/ep?x=qc",podcast: "[[PodNotes/Podcasts/My Show|My Show]]",tags: [podcastEpisode],status: null,rating: null,favorite: false; the raw title sits only in the body H1.date: null.Review
Ran a multi-dimension self-review plus two adversarial reviewers. Key fixes that came out of it: the whole-note migration gate (so an intentional empty field is never re-enabled), correcting the example
.baseto the documentedgroupByform (the canonical Bases schema has nosortview key), and qualifying the "valid YAML" claim for hand-edited feed-note paths that contain YAML-hostile characters.