Skip to content

perf(packages/core): incremental sync engine — only process what changed#76

Open
zrosenbauer wants to merge 17 commits intomainfrom
perf/incremental-sync-engine
Open

perf(packages/core): incremental sync engine — only process what changed#76
zrosenbauer wants to merge 17 commits intomainfrom
perf/incremental-sync-engine

Conversation

@zrosenbauer
Copy link
Copy Markdown
Member

Summary

  • Sync engine now skips unchanged pages, assets, images, and OpenAPI specs instead of running a full pipeline every pass
  • Page copy and public asset copy are parallelized via Promise.all
  • OpenAPI spec cache persists across dev-mode sync passes

Changes

File What changed
packages/core/src/sync/types.ts Extended Manifest, ManifestEntry, SyncContext with incremental metadata fields
packages/core/src/sync/copy.ts Added tryMtimeSkip + hashFrontmatter — short-circuits copyPage when source mtime and frontmatter hash match previous manifest
packages/core/src/sync/images.ts Skip copyFile when destination mtime >= source mtime
packages/core/src/banner/index.ts Parallelize generators, skip writes when content is identical
packages/core/src/sync/openapi.ts Track spec mtimes, skip SwaggerParser.dereference when mtime unchanged and cache hit
packages/core/src/sync/index.ts Parallelize page copy + copyAll, skip asset generation via config hash, compute skipMtimeOptimization from resolvedCount, store new manifest fields
packages/cli/src/commands/dev.ts Create and thread shared openapiCache
packages/cli/src/lib/watcher.ts Accept openapiCache, thread to sync(), clear on config reload

Test plan

  • pnpm check passes (typecheck + lint + format)
  • pnpm build passes
  • zpress dev — first sync runs full pipeline, second sync skips unchanged pages
  • Edit a markdown file → only that page reprocesses
  • Edit config → full resync (frontmatter hash mismatch triggers reprocess)
  • Add/remove a page → resolvedCount mismatch forces full resync for one pass, then resumes skipping
  • OpenAPI spec edit → spec re-parsed; no edit → cached

Skip unchanged pages via mtime + frontmatter hash, parallelize page copy
and public asset copy, cache OpenAPI spec parses across dev-mode syncs,
skip asset generation when config hash is unchanged, and skip image
re-copy when destination is current.

Co-Authored-By: Claude <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 31, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
oss-zpress Ready Ready Preview, Comment Apr 1, 2026 1:09am

Request Review

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 31, 2026

🦋 Changeset detected

Latest commit: d958675

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@zpress/core Minor
@zpress/cli Patch
@zpress/ui Patch
@zpress/kit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds an incremental sync engine: per-page mtime + frontmatter-hash skipping, content-hash write skipping, OpenAPI parse caching keyed by spec mtime, image copy skipping based on destination mtime, concurrent asset/page/public-asset copying, and structural-change detection via resolvedCount to force full resyncs. It refactors the dev command into a React/Ink DevScreen UI with watcher callbacks and a resync handle, introduces watcher types and manifest fields for incremental metadata, and adds extensive engine and CLI documentation.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: introducing incremental sync optimizations to process only what changed, which aligns directly with the core objective of the pull request.
Description check ✅ Passed The description is well-structured and directly related to the changeset, providing a clear summary of the incremental sync improvements, detailed file-by-file changes, and a concrete test plan.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

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


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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/cli/src/lib/watcher.ts (1)

43-49: ⚠️ Potential issue | 🟡 Minor

Document the new openapiCache parameter.

The exported createWatcher JSDoc still stops at onConfigReload, so callers cannot discover that this cache is shared across sync passes and cleared on config reload.

As per coding guidelines, "Every exported function needs explicit return types and full JSDoc."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/lib/watcher.ts` around lines 43 - 49, Update the exported
createWatcher function's JSDoc to document the new openapiCache parameter and
the function's return: add an `@param` entry for openapiCache on createWatcher
explaining it is an optional Map<string, unknown> that is shared across sync
passes and will be cleared when onConfigReload runs, and ensure there is an
explicit `@returns` description for the WatcherHandle return type; keep the JSDoc
adjacent to the createWatcher declaration and reference symbols createWatcher,
openapiCache, onConfigReload, and WatcherHandle so callers can discover the
behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/core/src/banner/index.ts`:
- Around line 115-139: The current mapping collapses per-asset errors to null
and always returns [null, written]; update the generator logic so actual errors
are propagated: instead of mapping to null on failure, detect and return the
first error (or reject) back to the caller—e.g. iterate generators sequentially
(for..of) or use Promise.allSettled on generators.map(...) and if any item has
an error from generate() or writeAsset(...) return that error as the first
element of the AssetResult (e.g. return [err, []]) rather than silently
returning [null, written]; refer to the generate() call, shouldGenerate(),
writeAsset(), and the final return to implement this early-error propagation.

In `@packages/core/src/sync/images.ts`:
- Around line 112-119: The freshness check using destStat.mtimeMs >=
exists.mtimeMs is unsafe; instead compute a content-based comparison and only
skip copying if the destination already has identical bytes. Read and hash the
source (absoluteImagePath) and destination (destPath) contents (e.g., using
crypto.createHash('sha256') or an equivalent checksum) and replace the mtime
comparison in the block around
imagePath/filename/destPath/absoluteImagePath/exists with a hash equality check;
if hashes differ (or dest missing), perform fs.copyFile as before.

In `@packages/core/src/sync/index.ts`:
- Around line 77-84: The current early-return based solely on
previousManifest.assetConfigHash can leave generated assets deleted in
options.paths.publicDir; update the skip logic in the function that computes
assetConfigChanged (referencing previousManifest, assetConfigHash, and
generateAssets) to also check for the presence of expected generated files
(e.g., "banner.svg", "logo.svg", "icon.svg") in options.paths.publicDir before
deciding to skip; if any of those files are missing, treat assetConfigChanged as
true and call generateAssets({ config: assetConfig, publicDir:
options.paths.publicDir }) so generate/copy logic (including copyAll) runs and
restores missing files.
- Around line 155-163: The skipMtimeOptimization check is only comparing page
count and misses structural changes in routing/output paths; compute and persist
a fingerprint of the source-to-output mapping (e.g. hash of the object returned
by buildSourceMap) and compare it against the previous manifest’s fingerprint
(add something like previousManifest.sourceMapHash); when the fingerprint
differs, set skipMtimeOptimization = false so copyPage() won’t early-return and
stale rewritten links are avoided; ensure the new fingerprint is stored on the
manifest and include sourceMap in copyCtx as before.

In `@packages/core/src/sync/openapi.ts`:
- Around line 159-162: The code mutates the shared cache via
ctx.openapiCache.set(specRelPath, parsed) which violates the project's
immutability rule; either stop mutating by returning an updated cache/map entry
from the parse function so the caller can replace ctx.openapiCache (e.g., return
{ parsed, updatedOpenapiCache }), or if you intend to keep the documented
side-effect, add an explicit comment and update the types/doc (types.ts) to mark
ctx.openapiCache as an intentional mutable boundary; locate the call in
openapi.ts (the ctx.openapiCache.set usage with specRelPath and parsed) and
implement one of those two options consistently.
- Around line 60-63: The current creation of specMtimes uses
Object.assign(...configResults.map(...)) with a type assertion; replace it with
a reduce over configResults to merge each result.specMtimes into an accumulator
(Record<string, number>) to avoid the unsafe cast and be more idiomatic. Locate
the specMtimes declaration and update it to initialize an empty Record<string,
number> accumulator and for each result in configResults copy or assign its
specMtimes entries into that accumulator (preserving numeric values), producing
a properly typed specMtimes without using a type assertion.

In `@packages/core/src/sync/types.ts`:
- Around line 40-48: The SyncContext currently exposes a mutable Map via the
openapiCache field; hide that internal mutation by removing openapiCache from
SyncContext and instead add a narrow abstraction (e.g., an OpenApiCache
interface or factory helpers) that exposes only needed ops such as get(path):
unknown | undefined, set(path, value): void, and clear(): void (or a
dereference(path) helper) and pass that abstraction through SyncContext; keep
the actual Map private to the sync orchestration implementation (initialize it
inside the sync runner) and update all usages that reference openapiCache to
call the new interface methods (referencing SyncContext and openapiCache symbol
names to locate changes).

---

Outside diff comments:
In `@packages/cli/src/lib/watcher.ts`:
- Around line 43-49: Update the exported createWatcher function's JSDoc to
document the new openapiCache parameter and the function's return: add an `@param`
entry for openapiCache on createWatcher explaining it is an optional Map<string,
unknown> that is shared across sync passes and will be cleared when
onConfigReload runs, and ensure there is an explicit `@returns` description for
the WatcherHandle return type; keep the JSDoc adjacent to the createWatcher
declaration and reference symbols createWatcher, openapiCache, onConfigReload,
and WatcherHandle so callers can discover the behavior.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 64e7c3be-81a2-4230-9930-81f578ef9602

📥 Commits

Reviewing files that changed from the base of the PR and between 850ef70 and 558bc35.

📒 Files selected for processing (9)
  • .changeset/incremental-sync-engine.md
  • packages/cli/src/commands/dev.ts
  • packages/cli/src/lib/watcher.ts
  • packages/core/src/banner/index.ts
  • packages/core/src/sync/copy.ts
  • packages/core/src/sync/images.ts
  • packages/core/src/sync/index.ts
  • packages/core/src/sync/openapi.ts
  • packages/core/src/sync/types.ts

Document the sync pipeline, page transformation, build/dev flows,
watcher triggers, and incremental sync optimizations.

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CONTRIBUTING.md`:
- Line 141: The fenced code block showing the flow "loadConfig() → sync() →
createRspressConfig() → rspress dev() → watcher" should include a language
specifier for consistent rendering; update the fence from ``` to a labeled one
such as ```text (or ```mermaid if supported) so the block uses a language
specifier and renders correctly.
- Line 133: The fenced flow diagram code block in CONTRIBUTING.md that shows the
sequence "loadConfig() → sync() → createRspressConfig() → rspress build() →
.zpress/dist/" is missing a language specifier; update that fenced block to
include a language (e.g., ```text or ```mermaid) so the flow diagram renders
consistently (choose `text` for plain text arrows or `mermaid` if you want
diagram support) and keep the existing content unchanged.
- Line 82: Add a language specifier to the fenced code block that contains the
directory listing (the triple-backtick fence immediately before the ".zpress/"
directory entry) so it renders consistently; change the opening fence from ```
to ```text (or ```bash) for that directory snippet to ensure proper formatting.
- Around line 145-154: The ignored-directories list in the CONTRIBUTING.md event
table is missing "bundle"; update the table row currently listing "node_modules,
.git, .zpress, dist, .turbo" to also include "bundle" so it accurately matches
the behavior implemented by createWatcher() (packages/cli/src/lib/watcher.ts)
which drops changes in that directory.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 033848ba-b3d3-43a0-8563-7717ecf6b91a

📥 Commits

Reviewing files that changed from the base of the PR and between 558bc35 and 69a3464.

📒 Files selected for processing (1)
  • CONTRIBUTING.md

Break `architecture.md` into three files:
- `architecture.md` — package ecosystem, layers, design decisions
- `sync-engine.md` — pipeline, page transformation, entry resolution,
  incremental sync, OpenAPI sync
- `config.md` — config system, output structure, Rspress integration

Update `cli.md` with corrected watcher details (fs.watch, not chokidar)
and dev lifecycle reflecting OpenAPI cache and Rspress restart behavior.

Revert CONTRIBUTING.md to its original state (architecture docs belong
in contributing/, not the root file).

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@contributing/concepts/config.md`:
- Around line 54-67: Add a language identifier to the directory-structure code
fence so markdown processors render it consistently; locate the fenced block
that begins with ".zpress/" and change the opening triple backticks to include
"text" (e.g., ```text) so the tree listing and comments render as plain
preformatted text.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 17855050-b79a-4c89-881b-52fb05357b8c

📥 Commits

Reviewing files that changed from the base of the PR and between 69a3464 and 58fd96f.

📒 Files selected for processing (5)
  • contributing/README.md
  • contributing/concepts/architecture.md
  • contributing/concepts/cli.md
  • contributing/concepts/config.md
  • contributing/concepts/sync-engine.md

zrosenbauer and others added 2 commits March 31, 2026 16:47
Replace the imperative dev command handler with an interactive
React/Ink screen component that renders live watcher status,
sync results, and keyboard shortcuts (r/c/o/q).

- Add DevScreen component with phase-based rendering
- Extract WatcherStatus, WatcherCallbacks, WatcherHandle types
- Return resolved port from startDevServer for TUI display
- Decouple watcher from Log interface via callbacks
- Add resync() to WatcherHandle for hotkey-triggered re-sync
- Cancel pending debounces on watcher close
- Guard all async state updates against unmount
- Wrap sync/server init in try/catch for proper error display

Co-Authored-By: Claude <noreply@anthropic.com>
- Replace Object.assign with reduce for type-safe specMtimes merge
- Add language specifiers to bare code fences in docs

Co-Authored-By: Claude <noreply@anthropic.com>
- Use function declaration instead of arrow for guard helper (func-style)
- Use Object.assign instead of spread in reduce accumulator (perf)

Co-Authored-By: Claude <noreply@anthropic.com>
The contributing docs had broken links to engine/*.md and
references/*.md because those directories were not included
in the zpress config glob patterns. This caused the Vercel
preview build to fail on link validation.

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@contributing/concepts/config.md`:
- Line 7: Update the documentation to remove the claim that validation happens
"at the boundary in defineConfig()" and that failures exit immediately: explain
that defineConfig() is a pass-through helper and actual validation occurs at
runtime in loadConfig(), and that validation failures return a Result-style
tuple rather than calling process.exit; reference the defineConfig() and
loadConfig() symbols so readers can find the implementation and adjust the
sentence to reflect runtime validation and Result-based error reporting.

In `@contributing/concepts/engine/dev.md`:
- Around line 54-64: Add a language identifier to the lifecycle steps fenced
code block that starts with "1. Parse args (`@kidd-cli/core`)" so it renders
consistently (e.g., change the opening fence from ``` to ```text). Locate the
fenced block containing the nine-step list in dev.md and update the opening
backticks to include the chosen language identifier while keeping the content
unchanged.

In `@contributing/concepts/engine/openapi.md`:
- Around line 50-54: Update Steps 1–2 wording to be precise: change Step 1 to
state configs are collected from the root config.openapi plus entry-level
openapi fields on entries (not that workspaces[] expose .openapi), and change
Step 2 to clarify that comparing spec mtime only suppresses dereference when a
shared OpenAPI cache (the Map<string, unknown> cache) persists across dev sync
passes; reference the incremental-sync behavior described in
incremental-sync-engine.md and the cache lifecycle in dev.md to ensure the text
matches the implemented fast path.

In `@contributing/concepts/engine/pipeline.md`:
- Around line 158-164: The explicit-items example uses the wrong key name;
update the example under "Explicit items" to use the actual schema key "title"
instead of "text" (i.e., show items entries as items: [{ title: '...', include:
'...' }]) so the documented config matches the validator and the schema that
expects title and include keys.
- Around line 77-89: Update the step table to use the engine's actual output
layout: change the "Create dirs" step to create `.zpress/content/` and
`.zpress/content/.generated/` (not `.generated/` at repo root); update "Generate
assets" and any other generated-metadata references to write into
`.zpress/content/.generated/`; change "Copy public/" to explicitly state copying
from `.zpress/public/` into the content root (i.e., into `.zpress/content/`);
and update "Write workspace data" to serialize to
`.zpress/content/.generated/workspaces.json` so all paths match the layout
described in contributing/concepts/config.md.

In `@packages/cli/src/commands/dev-screen.tsx`:
- Around line 81-91: The current try/catch assumes sync(...) throws; update this
block to handle a Result-style return if sync is refactored to return
Result<SyncResult, Error>: call const initialResult = await sync(config, {
paths, quiet: true, openapiCache }) and then pattern-match on the result (e.g.,
result.isOk()/result.ok or result.isErr()/result.error or a match method
depending on your Result type) instead of relying on throw — on success call
guard(setLastSync)(result.value) (or result.ok), on failure call
guard(setErrorMessage)(`Initial sync failed: ${toError(result.error).message}`)
and guard(setPhase)('error'), preserving the existing cancelled check and return
behavior.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8acde8f1-a6e1-4dc0-ba2a-931ed5a16589

📥 Commits

Reviewing files that changed from the base of the PR and between 58fd96f and 6e9e300.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !pnpm-lock.yaml
📒 Files selected for processing (19)
  • contributing/README.md
  • contributing/concepts/architecture.md
  • contributing/concepts/cli.md
  • contributing/concepts/config.md
  • contributing/concepts/engine/dev.md
  • contributing/concepts/engine/incremental.md
  • contributing/concepts/engine/openapi.md
  • contributing/concepts/engine/overview.md
  • contributing/concepts/engine/pipeline.md
  • contributing/guides/getting-started.md
  • contributing/references/cli.md
  • packages/cli/package.json
  • packages/cli/src/commands/dev-screen.tsx
  • packages/cli/src/commands/dev.ts
  • packages/cli/src/lib/dev-types.ts
  • packages/cli/src/lib/rspress.ts
  • packages/cli/src/lib/watcher.ts
  • packages/cli/tsconfig.json
  • packages/config/schemas/schema.json
💤 Files with no reviewable changes (1)
  • contributing/concepts/cli.md

- Fix config.md: defineConfig() is pass-through, validation in loadConfig()
- Fix dev.md: add `text` language identifier to lifecycle code fence
- Fix openapi.md: tighten cache/config wording in steps 1-2
- Fix pipeline.md: correct .generated/ paths to .zpress/content/.generated/
- Fix pipeline.md: correct public/ copy description
- Fix pipeline.md: use `title` instead of `text` in explicit items example

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

♻️ Duplicate comments (1)
packages/cli/src/commands/dev-screen.tsx (1)

83-93: 🛠️ Refactor suggestion | 🟠 Major

Duplicate: try/catch flow still depends on thrown errors instead of Result tuples.

This mirrors the previously raised concern in this PR thread: error control flow here is still throw/catch-driven for sync / startDevServer. If/when those APIs are Result-based, this should switch to tuple/result matching instead of catching exceptions.

As per coding guidelines, **/*.{ts,tsx} requires: “Use Result tuples for error handling instead of throw statements.”

Also applies to: 95-133

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/dev-screen.tsx` around lines 83 - 93, The current
try/catch around sync relies on thrown exceptions; change it to use the Result
tuple pattern: call sync and destructure the returned tuple (e.g. const [err,
initialResult] = await sync(...)), check cancelled, if err call
guard(setErrorMessage)(`Initial sync failed: ${toError(err).message)}`) and
guard(setPhase)('error') and return, otherwise call
guard(setLastSync)(initialResult). Apply the same conversion for the
startDevServer flow referenced in the block around startDevServer so both flows
handle [err, result] tuples instead of relying on exceptions; use the same
identifiers (sync, startDevServer, setLastSync, setErrorMessage, setPhase,
toError) to locate and update the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@contributing/concepts/engine/overview.md`:
- Line 71: Replace the phrasing "get their own namespace" with "use distinct
namespaces" in the Multi-sidebar description to tighten redundancy; update the
sentence that currently reads "**Multi-sidebar** -- Root entries share `/`,
isolated sections get their own namespace (e.g., `/apps/api/`)." to
"**Multi-sidebar** -- Root entries share `/`, isolated sections use distinct
namespaces (e.g., `/apps/api/`)."

In `@packages/cli/src/commands/dev-screen.tsx`:
- Line 84: The call to sync hard-codes quiet: true which ignores
DevScreenProps.quiet; update the call in DevScreen (where initialResult is
created) to pass the actual prop (e.g., quiet or props.quiet) instead of true so
the component honors the provided quiet flag, and if the prop is unused
elsewhere remove quiet from DevScreenProps and any callers to keep the API
consistent; locate the sync invocation and the DevScreenProps type to make the
corresponding change.
- Around line 52-53: Replace the mutable top-level flag declared as "let
cancelled = false" with a React ref: create a const cancelledRef = useRef(false)
and update all uses of the flag to read/write cancelledRef.current (including
where it was checked to prevent state updates and in cleanup logic where it is
set true); do the same replacement for the second occurrence referenced in the
review so the unmount-guard pattern remains but no mutable "let" is used.

In `@packages/core/src/sync/openapi.ts`:
- Around line 151-173: The catch for SwaggerParser.dereference currently returns
null on failure but leaves any previous parsed spec in ctx.openapiCache, so add
eviction there: inside the .catch((error) => { ... }) block, after computing the
message and before returning null, check if ctx.openapiCache is present and call
its delete/remove method for specRelPath (the same key used in
ctx.openapiCache.set), so stale specs are removed when SwaggerParser.dereference
(the failing call) fails.
- Around line 60-64: Replace the mutating Object.assign pattern inside the
reduce that builds specMtimes: in the configResults.reduce call that currently
uses Object.assign(acc, result.specMtimes) update the reducer to return a new
object via the spread operator (e.g., {...acc, ...result.specMtimes}) so
specMtimes is built immutably without mutating the accumulator.
- Around line 135-149: The current cache validation in openapi.ts uses only the
root spec mtime (specMtime) and ctx.previousManifest.openapiMtimes[specRelPath],
which can miss changes in files referenced via $ref; implement optional
referenced-file fingerprinting as a follow-up by (1) computing a deterministic
fingerprint (e.g., hash) of all resolved referenced file paths during
dereference in the same pipeline that produces the dereferenced spec, (2)
storing that fingerprint alongside openapiMtimes in the manifest entry for
specRelPath (e.g., extend previousManifest to include
openapiFingerprints[specRelPath]), and (3) when deciding to return
ctx.openapiCache.get(specRelPath) in the cache check (the block using specMtime,
ctx.openapiCache, and prevMtime), also compare the stored fingerprint against
the newly computed fingerprint and invalidate the cache if they differ so the
dereferenced output is regenerated.

---

Duplicate comments:
In `@packages/cli/src/commands/dev-screen.tsx`:
- Around line 83-93: The current try/catch around sync relies on thrown
exceptions; change it to use the Result tuple pattern: call sync and destructure
the returned tuple (e.g. const [err, initialResult] = await sync(...)), check
cancelled, if err call guard(setErrorMessage)(`Initial sync failed:
${toError(err).message)}`) and guard(setPhase)('error') and return, otherwise
call guard(setLastSync)(initialResult). Apply the same conversion for the
startDevServer flow referenced in the block around startDevServer so both flows
handle [err, result] tuples instead of relying on exceptions; use the same
identifiers (sync, startDevServer, setLastSync, setErrorMessage, setPhase,
toError) to locate and update the code.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 85f7d04c-d20f-4f27-983a-5a1fec02aa39

📥 Commits

Reviewing files that changed from the base of the PR and between 6e9e300 and 816d335.

📒 Files selected for processing (5)
  • CONTRIBUTING.md
  • contributing/concepts/engine/overview.md
  • packages/cli/src/commands/dev-screen.tsx
  • packages/core/src/sync/openapi.ts
  • zpress.config.ts

- Fix overview.md: tighten "get their own namespace" wording
- Fix dev-screen.tsx: replace `let cancelled` with useRef-backed flag
- Fix dev-screen.tsx: honor `quiet` prop instead of hard-coding true
- Fix openapi.ts: use immutable spread instead of Object.assign mutation
- Fix openapi.ts: evict stale cache entry on dereference failure

Co-Authored-By: Claude <noreply@anthropic.com>
…isable

The oxlint `no-accumulating-spread` rule flags spreading in reduce
for O(n^2) performance. Object.assign mutating the accumulator is the
correct pattern here — restored with a targeted lint disable comment.

Co-Authored-By: Claude <noreply@anthropic.com>
The dev-screen.tsx component uses JSX but the Rslib bundler was
defaulting to the classic React.createElement transform (ignoring
tsconfig's jsx: react-jsx). Configure SWC's react transform to use
the automatic runtime so jsx/jsxs are imported from react/jsx-runtime.

Co-Authored-By: Claude <noreply@anthropic.com>
Rspack's file-based storage layer holds a transaction lock on .temp/
inside the cache directory. Rsbuild's close() resolves before that
lock is fully released, causing the new dev() call to panic with
"Transaction already in progress". Add a 500ms settle window after
close to let the lock release before starting the new instance.

Co-Authored-By: Claude <noreply@anthropic.com>
Replace themeConfig.sidebar/nav with per-directory _meta.json and
root _nav.json files, enabling Rspress's built-in HMR for sidebar
and navigation changes without dev server restarts.

- Add filesystem-first _meta.json generation (meta.ts) that derives
  placement from actual file paths rather than config tree position
- Add write-meta.ts to write _meta.json per directory and _nav.json
- Remove old multi.ts sidebar builder and its tests
- Strip sidebar/nav from themeConfig in UI config
- Gate dev server restarts to config changes that actually require
  a rebuild (title, theme, colors, etc.) via restartRelevantHash
- Suppress Rspress dev output (logLevel: silent, progressBar: false)
- Keep sidebar.json/nav.json as .generated/ snapshots for debugging

Co-Authored-By: Claude <noreply@anthropic.com>
Rspress creates per-directory sidebar scopes by default when only
_nav.json is present. Adding a root _meta.json that lists all
top-level sections as dir items creates a single unified sidebar
keyed by "/" — matching the expected behavior.

Also simplifies the sidebar.json snapshot to a single "/" key
since standalone sections now share the unified sidebar.

Co-Authored-By: Claude <noreply@anthropic.com>
… sections

Rspress's _meta.json system only supports unified or per-directory sidebars,
not both. This adds a custom Sidebar component that filters the unified "/"
sidebar at runtime to isolate standalone sections (Packages, Contributing)
into their own scope while keeping the main 5 sections as a shared sidebar.

- Sync engine writes .generated/scopes.json with standalone section paths
- Config loads scopes and passes via themeConfig.standaloneScopePaths
- Custom Sidebar component filters unified sidebar data by current route
- Reimplements Rspress's createInitialSidebar collapsed state logic
- HMR preserved: _meta.json auto-discovery still runs with addDependency()

Co-Authored-By: Claude <noreply@anthropic.com>
Uses the kidd-cli `fullscreen: true` screen option to render the dev
TUI in the terminal's alternate screen buffer, preserving scrollback.

Co-Authored-By: Claude <noreply@anthropic.com>
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.

1 participant