feat(prompts): say what a run-time prompt is asking for and where it goes - #1555
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds prompt scopes and run context across formatters, engines, preflight scanning, input modals, and draft persistence. Prompts derive contextual labels, destinations, and stable draft identifiers, while template and capture flows propagate scope information through nested formatting. ChangesPrompt Context and Formatting
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ChoiceEngine
participant CompleteFormatter
participant InputPrompt
participant DraftStore
ChoiceEngine->>CompleteFormatter: set prompt scope and run context
CompleteFormatter->>InputPrompt: build scoped title, placeholder, and destination context
InputPrompt->>DraftStore: read or write draft using draftScopeId
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Deploying quickadd with
|
| Latest commit: |
b5ea97d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://09bcfaa1.quickadd.pages.dev |
| Branch Preview URL: | https://chhoumann-1546-prompt-contex.quickadd.pages.dev |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c0c8ada85
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/preflight/collectChoiceRequirements.ts (1)
297-329: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign template preflight scanning with runtime prompt order.
Runtime resolves the template path before folder and note-title formatting, but this collector scans the title/folder first. A shared anonymous
{{VALUE}}therefore gets preflight copy such as “Note title” even when the sequential flow first asks for a template path. Scan the template-path string first, while deferring its literal template-body scan until its runtime-equivalent position, and add a regression test covering shared{{VALUE}}across these scopes.As per coding guidelines, “
**/*.{test,spec}.{ts,tsx}: Add regression coverage for bug fixes.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/preflight/collectChoiceRequirements.ts` around lines 297 - 329, Update the preflight flow around scanContentWithTemplateIncludes and scanTemplateSource to match runtime order: scan the template-path string before folder and note-title formatting, but defer scanning the template body until its runtime-equivalent position. Add regression coverage in the relevant test file for a shared anonymous VALUE across template path, folder, and note-title scopes, verifying the preflight prompt assignment follows runtime order.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/engine/applyTemplateToActiveNote.test.ts`:
- Line 30: Update the setPromptRunContext mock in the applyTemplateToActiveNote
tests to capture its payload, then assert the expected choice/template draft
scope and destination for both choice-backed and bare-template application
paths.
In `@src/engine/templateEngine-title.test.ts`:
- Around line 15-22: Reindent the added mock members in the withPromptScope
setup, including its parameters and callback body, with tabs instead of spaces
to comply with the repository’s formatting conventions; preserve the existing
mock behavior.
---
Outside diff comments:
In `@src/preflight/collectChoiceRequirements.ts`:
- Around line 297-329: Update the preflight flow around
scanContentWithTemplateIncludes and scanTemplateSource to match runtime order:
scan the template-path string before folder and note-title formatting, but defer
scanning the template body until its runtime-equivalent position. Add regression
coverage in the relevant test file for a shared anonymous VALUE across template
path, folder, and note-title scopes, verifying the preflight prompt assignment
follows runtime order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 92879568-3516-4704-b2fb-d6c943259259
📒 Files selected for processing (42)
src/engine/CaptureChoiceEngine.audit-capture.test.tssrc/engine/CaptureChoiceEngine.heading-picker.test.tssrc/engine/CaptureChoiceEngine.merge.test.tssrc/engine/CaptureChoiceEngine.notice.test.tssrc/engine/CaptureChoiceEngine.selection.test.tssrc/engine/CaptureChoiceEngine.tssrc/engine/MacroChoiceEngine.tssrc/engine/SingleTemplateEngine.tssrc/engine/TemplateChoiceEngine.audit-template.test.tssrc/engine/TemplateChoiceEngine.collision.test.tssrc/engine/TemplateChoiceEngine.discovery.test.tssrc/engine/TemplateChoiceEngine.folderSorting.test.tssrc/engine/TemplateChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.tssrc/engine/TemplateInsertEngine.audit-template.test.tssrc/engine/TemplateInsertEngine.test.tssrc/engine/TemplateInsertEngine.tssrc/engine/applyTemplateToActiveNote.test.tssrc/engine/applyTemplateToActiveNote.tssrc/engine/runTemplateFromFolder.test.tssrc/engine/runTemplateFromFolder.tssrc/engine/templateEngine-title.test.tssrc/formatters/captureChoiceFormatter-clipboard.test.tssrc/formatters/captureChoiceFormatter.tssrc/formatters/completeFormatter.audit-formatter-core.test.tssrc/formatters/completeFormatter.imagePaste.test.tssrc/formatters/completeFormatter.promptContext.test.tssrc/formatters/completeFormatter.test.tssrc/formatters/completeFormatter.tssrc/formatters/formatter.tssrc/formatters/promptScope.test.tssrc/formatters/promptScope.tssrc/gui/GenericInputPrompt/GenericInputPrompt.tssrc/gui/GenericWideInputPrompt/GenericWideInputPrompt.tssrc/gui/promptContextLine.tssrc/preflight/RequirementCollector.tssrc/preflight/collectChoiceRequirements.tssrc/styles.csssrc/types/inputPrompt.tssrc/utils/InputPromptDraftStore.test.tssrc/utils/InputPromptDraftStore.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/engine/applyTemplateToActiveNote.test.ts`:
- Around line 302-321: Update the test case around applyTemplateToNote to invoke
templates/tpl.md twice before applying templates/other.md, then assert that the
captured draftScopeId values contain two consecutive
"template-insert#templates/tpl.md" entries followed by the distinct
other-template scope. Keep the existing scope collection and ensure the test
directly verifies repeated applications reuse the same scope.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 20fd0aba-0ff1-4261-b217-76102c51f596
📒 Files selected for processing (1)
src/engine/applyTemplateToActiveNote.test.ts
74db43b to
da720dc
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/engine/MacroChoiceEngine.ts`:
- Around line 868-871: Update the Open File handling around
formatter.formatFileName in MacroChoiceEngine to initialize the prompt run
context before formatting command.filePath. Include the choiceName and a stable
draftScopeId derived per command, ensuring multiple Open File commands within
one macro receive distinct stable identifiers while preserving the existing
filePath scope.
In `@src/formatters/promptScope.ts`:
- Around line 228-240: Update elideMiddlePath so long two-segment paths such as
Folder/filename are shortened while preserving the final filename instead of
returning the original path. Adjust the segments.length <= 2 handling to retain
the destination segment and apply the existing elision behavior, while
preserving unchanged results for paths already within maxLength and existing
multi-segment paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d7a1e43d-65d6-4472-8ebe-0dac04a4f7de
📒 Files selected for processing (42)
src/engine/CaptureChoiceEngine.audit-capture.test.tssrc/engine/CaptureChoiceEngine.heading-picker.test.tssrc/engine/CaptureChoiceEngine.merge.test.tssrc/engine/CaptureChoiceEngine.notice.test.tssrc/engine/CaptureChoiceEngine.selection.test.tssrc/engine/CaptureChoiceEngine.tssrc/engine/MacroChoiceEngine.tssrc/engine/SingleTemplateEngine.tssrc/engine/TemplateChoiceEngine.audit-template.test.tssrc/engine/TemplateChoiceEngine.collision.test.tssrc/engine/TemplateChoiceEngine.discovery.test.tssrc/engine/TemplateChoiceEngine.folderSorting.test.tssrc/engine/TemplateChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.tssrc/engine/TemplateEngine.tssrc/engine/TemplateInsertEngine.audit-template.test.tssrc/engine/TemplateInsertEngine.test.tssrc/engine/TemplateInsertEngine.tssrc/engine/applyTemplateToActiveNote.test.tssrc/engine/applyTemplateToActiveNote.tssrc/engine/runTemplateFromFolder.test.tssrc/engine/runTemplateFromFolder.tssrc/engine/templateEngine-title.test.tssrc/formatters/captureChoiceFormatter-clipboard.test.tssrc/formatters/captureChoiceFormatter.tssrc/formatters/completeFormatter.audit-formatter-core.test.tssrc/formatters/completeFormatter.imagePaste.test.tssrc/formatters/completeFormatter.promptContext.test.tssrc/formatters/completeFormatter.test.tssrc/formatters/completeFormatter.tssrc/formatters/formatter.tssrc/formatters/promptScope.test.tssrc/formatters/promptScope.tssrc/gui/GenericInputPrompt/GenericInputPrompt.tssrc/gui/GenericWideInputPrompt/GenericWideInputPrompt.tssrc/gui/promptContextLine.tssrc/preflight/RequirementCollector.tssrc/preflight/collectChoiceRequirements.tssrc/styles.csssrc/types/inputPrompt.tssrc/utils/InputPromptDraftStore.test.tssrc/utils/InputPromptDraftStore.ts
🚧 Files skipped from review as they are similar to previous changes (30)
- src/gui/promptContextLine.ts
- src/types/inputPrompt.ts
- src/engine/TemplateInsertEngine.audit-template.test.ts
- src/formatters/completeFormatter.imagePaste.test.ts
- src/engine/CaptureChoiceEngine.audit-capture.test.ts
- src/formatters/captureChoiceFormatter-clipboard.test.ts
- src/engine/TemplateInsertEngine.test.ts
- src/engine/CaptureChoiceEngine.notice.test.ts
- src/gui/GenericWideInputPrompt/GenericWideInputPrompt.ts
- src/utils/InputPromptDraftStore.ts
- src/engine/TemplateChoiceEngine.audit-template.test.ts
- src/engine/runTemplateFromFolder.ts
- src/engine/SingleTemplateEngine.ts
- src/engine/CaptureChoiceEngine.selection.test.ts
- src/formatters/captureChoiceFormatter.ts
- src/engine/TemplateInsertEngine.ts
- src/engine/TemplateChoiceEngine.notice.test.ts
- src/preflight/RequirementCollector.ts
- src/engine/applyTemplateToActiveNote.test.ts
- src/engine/TemplateChoiceEngine.discovery.test.ts
- src/formatters/promptScope.test.ts
- src/utils/InputPromptDraftStore.test.ts
- src/formatters/completeFormatter.audit-formatter-core.test.ts
- src/engine/TemplateChoiceEngine.ts
- src/engine/applyTemplateToActiveNote.ts
- src/formatters/completeFormatter.promptContext.test.ts
- src/formatters/completeFormatter.test.ts
- src/engine/TemplateEngine.ts
- src/engine/CaptureChoiceEngine.heading-picker.test.ts
- src/formatters/completeFormatter.ts
A choice's input prompt was a modal titled with the choice name over a single empty box. A Template's title prompt and a Capture's text prompt were pixel-identical, and nothing said where the answer would land (#1546). The choice name only ever reached the modal by accident: `CompleteFormatter.valueHeader` was set exclusively by `formatFileName(input, choiceName)` and never reset, so it leaked from the path pass into the later content pass. Capture-to-active-file, which runs no path pass, got a bare "Enter value"; MacroChoiceEngine passed "", which survives `?? "Enter value"` and rendered an EMPTY title. Replace it with a declared per-pass `PromptScopeKind` plus a run context (which choice, and the destination once resolved): - title = what is being asked, but ONLY when the answer is provably the whole of what the scope names. Paths use the strict rule (their literals are part of the answer); content scopes tolerate literal decoration, so `- [ ] {{VALUE}}` still asks for "Text to capture" while `{{DATE:YYYY-MM-DD}} {{VALUE}}` keeps the choice name rather than inviting the user to retype the date. - a muted context line under the title: choice name (when it is not already the title) and the resolved destination, with long paths elided in the middle so the file name survives. - the placeholder always names the field. The scope is declared by the caller, never inferred from which format* method ran: `formatFileContent` is shared by template bodies, capture bodies, the script API and the AI agent. Input drafts gain a `scopeId` so converging titles cannot make one choice's cancelled draft pre-fill another's, and an included `{{TEMPLATE:}}` prompt cannot inherit its parent's answer. The folder-template flow's ephemeral choice gets a path-derived id instead of a fresh uuid, so its draft key is stable across runs.
The line is clipped to one row and long paths are elided in the middle, so the shortened text was also all the tooltip offered. Carry the un-elided form alongside it.
…ntext
Five confirmed defects from a review of the change:
- A template BODY's literal text is the note, so one {{VALUE}} in
`---\nclient: {{VALUE}}\n---\n# Onboarding` was enough to title the
prompt "Note content" while it was really asking for a client name.
Only a capture format keeps the relaxed rule; its literals really are
one line of decoration.
- "Append to top/bottom" collisions build their own TemplateInsertEngine
(own formatter), which never received the run context: every prompt
raised while appending fell back to "Enter value" with no context line
and an unscoped draft key.
- Apply-template reconciliation reused the applied note's path as the
destination while asking where the note should MOVE to.
- A phone-width modal clipped the context line, cutting off exactly the
destination the line exists to show. It now wraps to two lines there,
and the hover tooltip is always set rather than only when the path was
elided.
- One-page inputs labelled the same field "Enter value" while the
sequential prompt called it "Note title"; the preflight collector now
shares the same scope-derived copy.
- Preflight only scanned a Template's file-name format when the toggle
was on, but the engine resolves a disabled format to {{VALUE}} anyway.
The one-page form therefore described the note TITLE using whatever
the template body said, contradicting the sequential prompt. It now
scans the effective format (and the same for a disabled capture
format), which also makes the implicit note-name prompt visible to the
non-interactive CLI guard for the first time.
- A {{TEMPLATE:}} include renders through its own formatter, which
hard-coded the note-body scope: an include spliced into a file name
claimed to be asking for note content, and offered clipboard-image
paste into a path. It now inherits the including pass's scope, and
image paste follows the declared sink rather than the pass alone.
- Apply-template set the destination before resolving the template
SOURCE path, so a token in that path was told it would land in the
note being templated. The destination is now set after the source
path resolves.
A prompt inside a template SOURCE path picks which template file to read, so the note being written is not where that answer lands. The context line now withholds the destination for that scope (and for the unknown "generic" scope) instead of each engine having to order its setPromptRunContext calls around the resolution, which had already been missed once in Capture's create-with-template path. Also reverts the preflight "scan the effective format" change from the previous commit. It was aimed at a label mismatch on the opt-in one-page surface, but collecting a note-title requirement the engine can satisfy from the editor selection broke two things: the non-interactive CLI guard began rejecting a stock Capture that would have captured the selection, and the one-page form began showing an empty title field where the selection used to fill it in, so an untouched submit stored "" and the run died with "File name is empty after formatting". Closing that properly needs the editor selection modelled in preflight for Template choices, which is a separate change.
With folder settings off, a formatted name whose first segment is an
existing root folder routes the note from the vault root instead of
Obsidian's default location - and the answer that reroutes it is the one
being typed. The previous guard only caught a literal slash in the
format, so `{{VALUE}}` answered `Projects/Note` (or a global variable
expanding to `Projects/`) still showed the default folder.
shouldTreatFormattedNameAsVaultRelativePath returns false as soon as the
folder is enabled, so "a folder is configured" is exactly the condition
under which the folder shown is guaranteed.
- A macro's Open file command builds its own formatter per command, so a
{{VALUE}} in that path got no context line and an unscoped draft key.
It now receives the macro's name and a per-command draft scope, so two
Open file commands in one macro cannot share a draft.
- elideMiddlePath returned a two-segment path untouched, leaving CSS to
clip the file name off the end - the opposite of the function's point.
A single folder is now dropped so the name survives.
571583a to
b5ea97d
Compare
Closes #1546.
The problem
A choice's run-time prompt was a modal titled with the choice name over one empty box. A Template's title prompt and a Capture's text prompt were pixel-identical, and nothing said where the answer would land.
Even the choice name only reached the modal by accident.
CompleteFormatter.valueHeaderwas set exclusively byformatFileName(input, choiceName)and never reset, so it leaked from the path pass into the later content pass. Two consequences fell out of that:Enter value.MacroChoiceEnginepassed"", which survives?? "Enter value", so a macro's Open file prompt had an empty modal title.The design
Every prompt should answer three things: what am I typing, for which choice, and where does it land. The change gives the formatter a declared per-pass scope plus a run context, and fills three slots from them:
Three decisions carry it.
The scope is declared by the caller, never inferred from which
format*method ran.formatFileContentis shared by template bodies, capture bodies,quickAddApi.formatand the AI agent, so method-derived copy would be a lie in three of those four cases.The derived title is used only when the answer is provably the whole of what the scope names. Otherwise the choice name stays the title (today's behaviour) and only the placeholder names the field. For a file name format of
{{DATE:YYYY-MM-DD}} {{VALUE}}, a title reading "Note title" would invite the user to retype the date. A vague title is not a lie; an authoritative wrong one is.Paths use the strict rule, because their literals are part of the answer (
Daily/{{VALUE}}.md). A capture format is a one-line template whose literals are decoration, so- [ ] {{VALUE}}still asks for exactly "Text to capture" - which matters, because the "Add to task list" toggle rewrites every default capture into that shape.No new configuration. Author-side overrides (
{{VALUE:name}},|label:) are untouched, and named tokens keep titling themselves with their variable name; they only gain the context line.Copy
Tradeoffs
scopeId. Converging titles would otherwise have collapsed the input-draft key across choices: a cancelled "Add book" would pre-fill "Meeting note". It also separates a{{TEMPLATE:}}include's own prompt from its parent's within one run, and fixes the pre-existing collision between two choices sharing a name.folder-template:<path>), so it still cannot collide with a persisted choice's uuid.FuzzySuggestModalhas no title element, and|label:already drives their placeholder.Not doing
Known residuals
{{VALUE}}prompts once; the title reflects the folder pass, which runs first.Validation
Reproduced first, in this worktree's own Obsidian 1.13.0 instance: both prompts came up titled with the choice name, empty placeholder, no description.
Verified after, live, across the shapes that stress the rules:
{{VALUE}}{{DATE}} {{VALUE}}{{VALUE:title}}Books/{{VALUE}}.mdA three-prompt Template run reads:
Note title / Add book → Books/, thenauthor / Add book → Books/Dune.md, thenrating / Add book → Books/Dune.md.Long paths are elided in the middle so the file name survives (
Work/…/Weekly standup notes.md), with the full path as the hover tooltip. Measured on the running app: 526px on a desktop modal (no clipping), and wrapping to exactly two lines with no clipping at a 300px modal width under the phone-width rules. The wide (multi-line) prompt renders the same line.pnpm test(3895 passing),pnpm lint,tsc, andpnpm run buildare green.Review
The design and the implementation each went through adversarial review rounds; the reviewers' confirmed findings are folded in, including several that were regressions in my own first attempt:
{{VALUE}}in a structured body was enough to title the prompt "Note content".TemplateInsertEngine, which never received the run context.{{TEMPLATE:}}include hard-coded the note-body scope, so an include spliced into a file name claimed to ask for note content and offered clipboard-image paste into a path.Summary by CodeRabbit