diff --git a/llm-docs/preview-architecture.md b/llm-docs/preview-architecture.md index 7dd5128268..faa222b0bf 100644 --- a/llm-docs/preview-architecture.md +++ b/llm-docs/preview-architecture.md @@ -1,12 +1,13 @@ --- -main_commit: fc0cf88dc -analyzed_date: 2026-05-29 +main_commit: 16257efdc +analyzed_date: 2026-06-15 key_files: - src/command/preview/cmd.ts - src/command/preview/preview.ts - src/project/serve/serve.ts - src/project/serve/watch.ts - src/project/project-shared.ts + - src/project/types.ts - src/execute/jupyter/jupyter.ts - src/execute/engine.ts --- @@ -203,7 +204,12 @@ These need a dependency→consumer map in the watch list, not a per-file stat. | `engine` | Execution engine instance | Re-determined | | `target` | Execution target (includes `.quarto_ipynb` path) | Re-created by `target()` | | `metadata` | YAML front matter | Recomputed from markdown | -| `brand` | Resolved `_brand.yml` data | Re-loaded from disk | +| `brand` | Per-file brand resolved from the file's frontmatter `brand:` | Re-loaded from disk | + +The `brand` row here is the **per-file** resolution (`resolveBrand(fileName)`), keyed +on the file's own frontmatter `brand:` and invalidated each re-render by +`invalidateForFile`. It is distinct from the project-level `project.brandCache` +(see "Project-lifetime caches"), which is the cache that went stale in #14593. ### fullMarkdown freshness guard (added for #10392) @@ -235,6 +241,46 @@ Without step 1, the Jupyter engine's `target()` function sees the old file on di Called at project cleanup (preview exit). Delegates to `invalidateForFile()` for each cache entry, removing all transient files and clearing the cache. This is the final cleanup — `invalidateForFile()` handles per-render cleanup for individual files. +## Project-lifetime caches (outside `fileInformationCache`) + +Two render-affecting caches hang directly off `ProjectContext` (`src/project/types.ts`), +**not** inside the per-file `fileInformationCache`: + +| Field | Content | Lifetime | +|-------|---------|----------| +| `brandCache` | Project-level brand resolved from `_brand.yml` candidates | Write-once | +| `outputNameIndex` | Map of output name → input file + format | Write-once | + +Both are populated on first use and read forever after. Nothing in the preview +re-render path clears them — only a full context rebuild does +(`refreshProjectConfig` in `watch.ts` replaces the whole `ProjectContext` when a +config file changes). The per-file `invalidateForFile` does **not** touch them. + +### `brandCache` staleness (#14593) + +`projectResolveBrand` (`project-shared.ts`, the `fileName === undefined` branch) +populated `project.brandCache` on first resolve and early-returned it forever. In +a long-lived preview context a `_brand.yml` added or removed mid-session was +ignored until the process restarted. RStudio's "Render" button runs +`quarto preview --no-watch-inputs` over the same persistent context, so it +observed the same stale brand. + +Unlike a source edit, the render-request path carries **no change signal**: the +input `.qmd` is unchanged, and the watcher does not watch `_brand.yml` (a +separate watcher-coverage issue). Active invalidation has nothing to hook onto. The only +place that can notice the brand file appearing/disappearing is **at resolve time, +by checking the filesystem** — so the fix is a passive staleness guard inside +`projectResolveBrand`, mirroring the `fullMarkdown` mtime+size guard. + +The guard derives a *source-state token* over the candidate brand paths the +resolver already consults (the four `_brand.{yml,yaml}` / `_brand/_brand.{yml,yaml}` +defaults under `project.dir`, or the `brand:` string path, or the `{light,dark}` +paths) — combining each file's existence + mtime + size into one comparable +string. The cache is reused only when the freshly-computed token matches; on any +mismatch (add, remove, or content edit) the brand is re-resolved and the new +token stored. Format-independent, since `projectResolveBrand` takes no format +argument — typst and HTML are both covered. + ## Transient Notebook Lifecycle (.quarto_ipynb) When rendering a `.qmd` with a Jupyter kernel, the engine creates a transient `.ipynb` notebook: diff --git a/news/changelog-1.10.md b/news/changelog-1.10.md index c4908d9192..92a4cf0b17 100644 --- a/news/changelog-1.10.md +++ b/news/changelog-1.10.md @@ -56,6 +56,7 @@ All changes included in 1.10: - ([#10392](https://github.com/quarto-dev/quarto-cli/issues/10392)): Fix `quarto preview` of a website or book project showing stale HTML for non-index pages after editing the source `.qmd`. - ([#14281](https://github.com/quarto-dev/quarto-cli/issues/14281)): Avoid creating a duplicate `.quarto_ipynb` file on preview startup for single-file Jupyter documents. - ([#14533](https://github.com/quarto-dev/quarto-cli/issues/14533)): Fix `quarto preview` not detecting a frontmatter `format:` change until the second render request. The first request after the edit now correctly restarts the preview process with the new format. +- ([#14593](https://github.com/quarto-dev/quarto-cli/issues/14593)): Fix `quarto preview` ignoring a `_brand.yml` added or removed while the preview is running. The brand change is now applied on the next render instead of requiring a preview restart. ### `install` diff --git a/src/project/project-shared.ts b/src/project/project-shared.ts index e17b4556ea..1eac8e6cc0 100644 --- a/src/project/project-shared.ts +++ b/src/project/project-shared.ts @@ -583,17 +583,20 @@ export async function projectResolveBrand( } return resolved; } + // A token over the candidate brand files' existence + mtime + size, so the + // long-lived preview brandCache is reused only while the on-disk brand state + // is unchanged. Mirrors the fullMarkdown mtime+size guard above. + function brandSourceState(paths: string[]): string { + return paths.map((path) => { + try { + const stat = Deno.statSync(path); + return `${path}:${stat.mtime?.getTime()}:${stat.size}`; + } catch { + return `${path}:absent`; + } + }).join("|"); + } if (fileName === undefined) { - if (project.brandCache) { - return project.brandCache.brand; - } - project.brandCache = {}; - let fileNames = [ - "_brand.yml", - "_brand.yaml", - "_brand/_brand.yml", - "_brand/_brand.yaml", - ].map((file) => join(project.dir, file)); const brand = (project?.config?.brand ?? project?.config?.project.brand) as | boolean @@ -602,6 +605,39 @@ export async function projectResolveBrand( light?: string; dark?: string; }; + + // Determine the candidate brand files this resolve will consult, so the + // staleness token below covers exactly the paths that affect the result. + let candidatePaths: string[]; + if ( + typeof brand === "object" && brand && + ("light" in brand || "dark" in brand) + ) { + candidatePaths = [ + brand.light ? resolveBrandPath(brand.light, project.dir) : undefined, + brand.dark ? resolveBrandPath(brand.dark, project.dir) : undefined, + ].filter((path): path is string => path !== undefined); + } else if (typeof brand === "string") { + candidatePaths = [join(project.dir, brand)]; + } else { + candidatePaths = [ + "_brand.yml", + "_brand.yaml", + "_brand/_brand.yml", + "_brand/_brand.yaml", + ].map((file) => join(project.dir, file)); + } + + // In preview mode the project context is long-lived and brandCache is + // reused across re-renders. Reuse the cached brand only when the on-disk + // state of the candidate files is unchanged; otherwise re-resolve so a + // _brand.yml added, removed, or edited mid-session takes effect (#14593). + const sourceState = brandSourceState(candidatePaths); + if (project.brandCache && project.brandCache.sourceState === sourceState) { + return project.brandCache.brand; + } + project.brandCache = { sourceState }; + if (brand === false) { project.brandCache.brand = undefined; return project.brandCache.brand; @@ -621,11 +657,8 @@ export async function projectResolveBrand( }; return project.brandCache.brand; } - if (typeof brand === "string") { - fileNames = [join(project.dir, brand)]; - } - for (const brandPath of fileNames) { + for (const brandPath of candidatePaths) { if (!existsSync(brandPath)) { continue; } diff --git a/src/project/types.ts b/src/project/types.ts index 8014c0d067..99a35574f0 100644 --- a/src/project/types.ts +++ b/src/project/types.ts @@ -87,8 +87,11 @@ export interface ProjectContext extends Cloneable { fileInformationCache: FileInformationCache; - // This is a cache of _brand.yml for a project - brandCache?: { brand?: LightDarkBrandDarkFlag }; + // This is a cache of _brand.yml for a project. sourceState is a token over + // the candidate brand files' existence + mtime + size, so a _brand.yml added, + // removed, or edited during a long-lived preview context invalidates the + // cache instead of serving a stale brand (#14593). + brandCache?: { brand?: LightDarkBrandDarkFlag; sourceState?: string }; resolveBrand: ( fileName?: string, ) => Promise< diff --git a/tests/docs/manual/preview/README.md b/tests/docs/manual/preview/README.md index 95f4a9dfc3..36b0c7d856 100644 --- a/tests/docs/manual/preview/README.md +++ b/tests/docs/manual/preview/README.md @@ -216,6 +216,58 @@ format edit is detected on the **first** render request. - **Expected:** No preview process restart. Render runs in the same process. - **Catches:** Over-eager invalidation triggering 404 when the format is in fact unchanged +## Test Matrix: Brand Detection (#14593) + +After every change to `projectResolveBrand` or the `project.brandCache` guard, +verify that a sibling `_brand.yml` added or removed **while a preview is running** +takes effect on the next render — without restarting the preview process. Fixture: +`brand-detection/` (`report.qmd`, `brand-imperial.yml`); drive it with the +`/quarto-preview-test` workflow. The +brand signal in the kept `report.typ` is the line +`#show link: set text(fill: rgb("#bc1e22")` (present ⇒ brand applied). See +`brand-detection/README.md` for the on/off check. + +### P1: Critical + +#### T28: Single .qmd — `_brand.yml` added mid-preview + +- **Setup:** `brand-detection/report.qmd` (`format: typst`, `keep-typ: true`), no `_brand.yml` present +- **Steps:** `quarto preview report.qmd --to typst --no-browser`, copy `brand-imperial.yml` to `_brand.yml`, force one re-render (e.g. touch `report.qmd`), grep `report.typ` +- **Expected:** `report.typ` gains the brand link-color line. Brand applies without restarting preview. +- **Catches:** Stale `project.brandCache` serving "no brand" after a `_brand.yml` appeared + +#### T29: Single .qmd — `_brand.yml` removed mid-preview + +- **Setup:** Same fixture with `_brand.yml` present and the link colored +- **Steps:** Remove `_brand.yml`, force one re-render, grep `report.typ` +- **Expected:** The brand link-color line disappears — output reverts to no-brand +- **Catches:** Stale `project.brandCache` holding the previous brand after the file was removed + +### P2: Important + +#### T30: HTML format — `_brand.yml` added mid-preview + +- **Setup:** `report.qmd` with `format: html`, no `_brand.yml` +- **Steps:** As T28, inspect the rendered HTML/CSS for the brand primary color +- **Expected:** Brand applies. `projectResolveBrand` is format-independent, so the same guard covers HTML. +- **Catches:** A fix that only worked for the typst code path + +#### T31: Project (`_quarto.yml`) — `_brand.yml` added/removed mid-preview + +- **Setup:** Website/book project with `_quarto.yml`, no project-level `_brand.yml` +- **Steps:** `quarto preview` the project, add then remove a `_brand.yml`, force re-renders between +- **Expected:** Brand applies on add and reverts on remove, both without restart +- **Catches:** The project (multi-file) `ProjectContext` sharing the same stale `brandCache` + +### Regression Guard + +#### T32: Unchanged `_brand.yml` — no spurious re-resolve cost + +- **Setup:** A preview with a stable `_brand.yml` +- **Steps:** Save the source repeatedly without touching `_brand.yml` +- **Expected:** Brand stays applied; the source-state token matches so the cache is reused (only cheap stat calls per resolve) +- **Catches:** A guard that re-loads the brand on every render even when nothing changed + ## Extension Format PDF Preview (#14582) ### P1: Core functionality diff --git a/tests/docs/manual/preview/brand-detection/README.md b/tests/docs/manual/preview/brand-detection/README.md new file mode 100644 index 0000000000..f49cd1e225 --- /dev/null +++ b/tests/docs/manual/preview/brand-detection/README.md @@ -0,0 +1,47 @@ +# Brand detection during preview (#14593) + +Manual fixture for the bug where `quarto preview` ignored a sibling `_brand.yml` +added or removed while the preview was running: `project.brandCache` was resolved +once and never invalidated, so the brand only took effect after restarting the +preview process. RStudio's "Render" button hits the same persistent preview +server, so it observed the same stale brand. + +Drive this with the `/quarto-preview-test` workflow (start preview, mutate +`_brand.yml`, force a re-render, inspect output). See the parent +`../README.md` "Test Matrix: Brand Detection (#14593)" for the T28–T32 cases. + +## Files + +| File | Role | +|------|------| +| `report.qmd` | Minimal Typst doc with a link; `keep-typ: true` so the `.typ` is inspectable | +| `brand-imperial.yml` | A brand setting `primary: imperial-red` (`#BC1E22`) | + +`report.typ`, `report.pdf`, `_brand.yml`, and `.quarto/` are scratch produced by a +run — only `report.qmd` and `brand-imperial.yml` are tracked. + +## Brand signal + +The brand is applied when the kept `report.typ` contains: + +``` +#show link: set text(fill: rgb("#bc1e22") +``` + +Present ⇒ brand ON (link colored). Absent ⇒ brand OFF. + +## Manual on/off check + +1. Start with no `_brand.yml` and preview the doc: + `quarto preview report.qmd --to typst --no-browser`. +2. **ON** — copy `brand-imperial.yml` to `_brand.yml`, force a re-render (touch + `report.qmd`), and confirm `report.typ` gains the brand link-color line. +3. **OFF** — remove `_brand.yml`, force another re-render, and confirm the + line disappears. + +Before the fix the brand stayed at its first-resolved state (OFF in the add case, +ON in the remove case) until the preview process was stopped. After the fix, the +resolve-time source-state guard on `project.brandCache` notices the `_brand.yml` +appear/disappear and re-resolves on the next render. A separate `quarto render` +(fresh context) always resolved the brand correctly, which is the disambiguator +that isolates the bug to the persistent preview context. diff --git a/tests/docs/manual/preview/brand-detection/brand-imperial.yml b/tests/docs/manual/preview/brand-detection/brand-imperial.yml new file mode 100644 index 0000000000..ce068b8aa7 --- /dev/null +++ b/tests/docs/manual/preview/brand-detection/brand-imperial.yml @@ -0,0 +1,4 @@ +color: + palette: + imperial-red: "#BC1E22" + primary: imperial-red diff --git a/tests/docs/manual/preview/brand-detection/report.qmd b/tests/docs/manual/preview/brand-detection/report.qmd new file mode 100644 index 0000000000..9c8256048e --- /dev/null +++ b/tests/docs/manual/preview/brand-detection/report.qmd @@ -0,0 +1,12 @@ +--- +title: "Brand detection during preview" +format: + typst: + keep-typ: true +--- + +Voir la [documentation](https://example.com). + +When a sibling `_brand.yml` sets `primary: imperial-red`, the link above +turns red in the Typst output. Adding or removing `_brand.yml` while a +`quarto preview` session is running is the behavior under test. diff --git a/tests/unit/preview-brand-cache.test.ts b/tests/unit/preview-brand-cache.test.ts new file mode 100644 index 0000000000..a77b4ff776 --- /dev/null +++ b/tests/unit/preview-brand-cache.test.ts @@ -0,0 +1,184 @@ +/* + * preview-brand-cache.test.ts + * + * Tests that projectResolveBrand re-resolves _brand.yml when the file is + * added or removed during the lifetime of a long-lived ProjectContext + * (as happens in `quarto preview`, where the context persists across + * re-renders). Regression test for quarto-cli-t7b1. + * + * The bug: project.brandCache is populated on first resolve and never + * invalidated, so a _brand.yml created/removed mid-session is ignored + * until the preview process restarts. RStudio's Render button runs + * `quarto preview --no-watch-inputs` and routes re-renders through the + * same persistent context, so it observes the stale brand. + * + * Copyright (C) 2026 Posit Software, PBC + */ + +import { unitTest } from "../test.ts"; +import { assert, assertEquals } from "testing/asserts"; +import { join } from "../../src/deno_ral/path.ts"; +import { singleFileProjectContext } from "../../src/project/types/single-file/single-file.ts"; +import { projectContext } from "../../src/project/project-context.ts"; +import { notebookContext } from "../../src/render/notebook/notebook-context.ts"; +import { initYamlIntelligenceResourcesFromFilesystem } from "../../src/core/schema/utils.ts"; +import { safeRemoveSync } from "../../src/core/path.ts"; + +const BRAND_YML = `color: + palette: + imperial-red: "#BC1E22" + primary: imperial-red +`; + +unitTest( + "projectResolveBrand - single-file: _brand.yml added mid-session is detected (t7b1)", + async () => { + await initYamlIntelligenceResourcesFromFilesystem(); + + const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" }); + const file = join(tmpDir, "test.qmd"); + const brandFile = join(tmpDir, "_brand.yml"); + + try { + Deno.writeTextFileSync(file, "---\ntitle: test\nformat: typst\n---\n"); + + const project = await singleFileProjectContext(file, notebookContext()); + + // First resolve with no _brand.yml: brand is undefined and the + // result is cached on the persistent project context. + const before = await project.resolveBrand(); + assertEquals(before, undefined, "no _brand.yml yet, brand must be undefined"); + + // User adds _brand.yml while the (preview) context is still alive. + Deno.writeTextFileSync(brandFile, BRAND_YML); + + // A subsequent resolve must pick up the newly-added brand. + const after = await project.resolveBrand(); + assert( + after !== undefined, + "brand must be resolved after _brand.yml is added (currently stale-cached as undefined)", + ); + } finally { + safeRemoveSync(tmpDir, { recursive: true }); + } + }, +); + +unitTest( + "projectResolveBrand - single-file: _brand.yml removed mid-session is detected (t7b1)", + async () => { + await initYamlIntelligenceResourcesFromFilesystem(); + + const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" }); + const file = join(tmpDir, "test.qmd"); + const brandFile = join(tmpDir, "_brand.yml"); + + try { + Deno.writeTextFileSync(file, "---\ntitle: test\nformat: typst\n---\n"); + Deno.writeTextFileSync(brandFile, BRAND_YML); + + const project = await singleFileProjectContext(file, notebookContext()); + + const before = await project.resolveBrand(); + assert(before !== undefined, "_brand.yml present, brand must resolve"); + + // User removes/deactivates _brand.yml mid-session (the `.bak` case). + Deno.removeSync(brandFile); + + const after = await project.resolveBrand(); + assertEquals( + after, + undefined, + "brand must revert to undefined after _brand.yml is removed (currently stale-cached as defined)", + ); + } finally { + safeRemoveSync(tmpDir, { recursive: true }); + } + }, +); + +unitTest( + "projectResolveBrand - project (_quarto.yml present): _brand.yml added mid-session is detected (t7b1)", + async () => { + await initYamlIntelligenceResourcesFromFilesystem(); + + const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" }); + const brandFile = join(tmpDir, "_brand.yml"); + let project; + + try { + // A real multi-file project with a _quarto.yml, no brand declared. + Deno.writeTextFileSync( + join(tmpDir, "_quarto.yml"), + "project:\n type: default\n", + ); + Deno.writeTextFileSync( + join(tmpDir, "index.qmd"), + "---\ntitle: test\nformat: typst\n---\n", + ); + + project = await projectContext(tmpDir, notebookContext()); + assert(project !== undefined, "projectContext must resolve for a _quarto.yml dir"); + + const before = await project.resolveBrand(); + assertEquals(before, undefined, "no _brand.yml yet, brand must be undefined"); + + Deno.writeTextFileSync(brandFile, BRAND_YML); + + const after = await project.resolveBrand(); + assert( + after !== undefined, + "brand must be resolved after _brand.yml is added to the project (currently stale-cached as undefined)", + ); + } finally { + // Release the project's disk cache before removing the dir, otherwise + // Windows holds a lock on the temp directory. + project?.cleanup?.(); + safeRemoveSync(tmpDir, { recursive: true }); + } + }, +); + +unitTest( + "projectResolveBrand - project (_quarto.yml present): _brand.yml removed mid-session is detected (t7b1)", + async () => { + await initYamlIntelligenceResourcesFromFilesystem(); + + const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" }); + const brandFile = join(tmpDir, "_brand.yml"); + let project; + + try { + // A real multi-file project with a _quarto.yml and a _brand.yml present. + Deno.writeTextFileSync( + join(tmpDir, "_quarto.yml"), + "project:\n type: default\n", + ); + Deno.writeTextFileSync( + join(tmpDir, "index.qmd"), + "---\ntitle: test\nformat: typst\n---\n", + ); + Deno.writeTextFileSync(brandFile, BRAND_YML); + + project = await projectContext(tmpDir, notebookContext()); + assert(project !== undefined, "projectContext must resolve for a _quarto.yml dir"); + + const before = await project.resolveBrand(); + assert(before !== undefined, "_brand.yml present, brand must resolve"); + + Deno.removeSync(brandFile); + + const after = await project.resolveBrand(); + assertEquals( + after, + undefined, + "brand must revert to undefined after _brand.yml is removed from the project (currently stale-cached as defined)", + ); + } finally { + // Release the project's disk cache before removing the dir, otherwise + // Windows holds a lock on the temp directory. + project?.cleanup?.(); + safeRemoveSync(tmpDir, { recursive: true }); + } + }, +);