Fix: startup indexing progress rendering

[ProudBenzene]

Fixes #183.

Changes

• Render startup progress as a single text node to prevent duplicated counts during Obsidian startup.
• Emit periodic progress events and briefly yield to the renderer so the counter stays responsive while indexing.

Testing

• Tested with a 1,751-file vault across full Obsidian restarts.
• Verified monotonic progress through 1751 / 1751.
• All 321 tests pass.

[Copilot]

Pull request overview

Fixes incorrect startup indexing progress rendering in Obsidian by avoiding multi-node JSX text reconciliation issues and by making progress updates more responsive during long initial indexing.

Changes:

• Reworks the startup progress UI to render/update the progress as a single DOM text node.
• Adds a periodic yield mechanism during indexing and emits index-progress events to keep the UI repainting.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/ui/loading-boundary.tsx	Switches progress rendering to a ref-driven single text node and subscribes to index-progress events for updates.
src/index/datacore.ts	Adds periodic yielding during initialization and triggers index-progress events to drive the loading UI.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[bitskc]

Local CI Results

PR: #184 | SHA: 937c0397f39f5a19f019deaad177135703214683 | Ran: 2026-08-16 10:09:43 UTC

Check	Status	Duration
Install dependencies	✅	2s
Tests (jest)	✅	10s
Format check (prettier)	❌	8s
Type check (tsc --noEmit)	✅	11s
Build (tsc + api-extractor + esbuild)	✅	7s

Overall: ❌ 1 check failed (pre-existing, not introduced by this PR)

Failures

yarn run prettier --check src

[warn] src/index/import/canvas.ts
[warn] Code style issues found in the above file(s). Forgot to run Prettier?
error Command failed with exit code 1.

Analysis: The formatting issue is in src/index/import/canvas.ts, which is not touched by this PR. The PR only modifies src/index/datacore.ts and src/ui/loading-boundary.tsx. This is a pre-existing formatting violation on master, not a regression introduced by PR #184. The two changed files both pass prettier.

Warnings

• Install: Pre-existing unmet peer dependency warnings for react/react-dom (the project uses preact via path aliases) and ts-jest@27 reporting an untested TypeScript 5.x version. Not related to this PR.
• Build: Pre-existing api-extractor ae-missing-release-tag warnings for many exported API symbols (missing @alpha/@beta/@public/@internal tags) and one tsdoc-code-fence-opening-indent warning. Not related to this PR; build completes successfully.
• Environment note (jest): The local runner uses Node 24 + jest 27, which triggers a stripAnsi is not a function crash in jest's string-length dependency (ESM-only strip-ansi@7 hoisted over the CJS strip-ansi@6 that string-length@4 expects). This is an environment/dependency-resolution incompatibility, not a PR code issue. A local node_modules patch (swapping in the CJS strip-ansi-cjs@6.0.1) was applied to run the suite; after the patch, all 321 tests across 17 suites passed. The repo's CI (GitHub Actions) runs on Node 18 where this does not occur.

PR Summary

PR #184 ("Fix: startup indexing progress rendering") makes the startup indexing progress UI responsive by:

• Adding periodic yields (YIELD_INTERVAL = 32 files, YIELD_DELAY_MS = 16ms) in DatacoreInitializer so Obsidian can repaint during initialization, and triggering an index-progress event on each yield.
• Replacing the debounced useIndexUpdates hook in LoadingProgress with a ref-callback that updates textContent directly on each index-progress event, giving immediate visible progress instead of a debounced 250ms lag.

All tests pass, types check, and the build succeeds. The only failing check (prettier) is a pre-existing issue in an unrelated file.

[bitskc]

Automated Code Review

Risk Level: Medium | Verdict: MINOR_CONCERNS
Recommendation: ⚠️ Merge with follow-up

Persona	Severity	Finding
Security Auditor	—	None. No new inputs, secrets, auth, or attack surface.
Regression Skeptic	P2	Final progress count may never render for vaults where targetTotal % 32 ∈ [1, 8] — the last in-flight batch completes during the yield window, suppressing the terminal index-progress event.
Performance Critic	P3	Concurrency drains to 0 on each 16 ms yield (in-flight batch completes, no new work starts until the timeout refills). Deliberate UX tradeoff; ~(files/32) × 16ms added wall time. Acceptable.
Architecture Judge	P3	window.setTimeout is not registered with the Component lifecycle (registerInterval), so it isn't cleared on unload; the callback is guarded by the active check so it's safe, just not clean. Also index-progress is fired via this.core.events.trigger(...) directly, bypassing the typed trigger overloads on Datacore.
QA Lead	P3	No automated coverage for the new yield/progress logic; the 321 passing tests are pre-existing. The manual test used 1,751 files (1751 % 32 == 23), which misses the small-vault edge case.
Product Reliability	P3	Pending setTimeout survives unload (≤16 ms, guarded by active → no-op). No observability gap; index-progress fires every 32 files.

Opened Issues

• #186 Startup progress counter can get stuck before targetTotal for small vaults

Summary

The PR cleanly fixes the duplicated-count rendering (single text node via a ref callback) and adds periodic yields so the loading counter repaints during long indexing — both reasonable and well-scoped. One real edge case: when the files remaining after the last periodic yield all finish inside the 16 ms yield window (targetTotal % 32 ∈ [1, 8]), the terminal index-progress event is suppressed and the counter sticks at the last multiple of 32 until the loading boundary dismisses. Cosmetic and transient, but it contradicts the PR's monotonic-progress goal; tracked in #186 with a minimal fix. Safe to merge with that follow-up.

[bitskc]

Local CI Results

PR: #184 | SHA: a4a990e4f614a71be434a213882a5cb9c101c98b | Ran: 2026-08-16 10:15:00 UTC

Check	Status	Duration
Install dependencies	✅	2s
Tests (jest)	✅	11s
Type check (tsc --noEmit)	✅	13s
Format check (prettier --check src)	❌	9s
Build (yarn run build)	✅	7s

Overall: ❌ 1 check failed (pre-existing, not introduced by this PR)

Summary

All 321 tests across 17 test suites pass. TypeScript type checking passes. Build completes successfully. The only failure is a pre-existing prettier formatting issue in src/index/import/canvas.ts — a file not modified by this PR. The PR's changed files (src/index/datacore.ts, src/ui/loading-boundary.tsx) both pass prettier formatting checks.

Failures

yarn run prettier --check src

[warn] src/index/import/canvas.ts
[warn] Code style issues found in the above file(s). Forgot to run Prettier?

Analysis: The formatting issue is in src/index/import/canvas.ts, which is not touched by this PR (PR only modifies src/index/datacore.ts and src/ui/loading-boundary.tsx). Verified that both PR-changed files pass prettier. This is a pre-existing issue on the base branch.

Warnings

• Node.js compatibility: Tests required a node_modules symlink workaround for Node 24 (jest 27's string-length dependency picks up the hoisted ESM-only strip-ansi@7 instead of the CJS strip-ansi-cjs@6). The repo's CI targets Node 18 where this is not an issue. This is an environment artifact, not a code problem.
• ts-jest version warning: TypeScript 5.8.3 has not been tested with ts-jest 27 (which expects <5.0.0). Non-fatal — all tests pass.
• api-extractor warnings: Numerous pre-existing TSDoc/release-tag warnings during build. Non-fatal — build completes successfully.
• esbuild bundle size warning: build/plugin/main.js is 1.3mb (⚠️). Pre-existing.

[bitskc]

Automated Code Review

Re-review — PR updated since the prior review (prior SHA 937c0397 → current HEAD a4a990e4). This comment covers the delta; the prior review's unchanged P3 tech-debt notes remain valid and are summarized below.

Risk Level: Medium | Verdict: CLEAN
Recommendation: ✅ Safe to merge

What changed in the delta (commit a4a990e4 — "fix: handle terminal startup progress update")

1. src/index/datacore.ts — terminal progress event. DatacoreInitializer.handleResult now checks this.initialized == this.targetTotal first and fires index-progress + runNext() before the periodic-yield branch. This directly resolves the prior P2 finding ([Auto-Detected] Startup progress counter can get stuck before targetTotal for small vaults #186): the counter now always reaches targetTotal / targetTotal even when the final in-flight batch completes during the 16 ms yield window. Ordering (terminal before yield) is correct — completion can no longer be swallowed by a pending yield.
2. src/ui/loading-boundary.tsx — datacore.offref. The ref-callback cleanup changed from datacore.events.offref(...) to datacore.offref(...). Both are valid (Events exposes offref at obsidian.d.ts:1482, and Datacore.offref delegates to it); this is a cosmetic/defensive change with no behavior difference.

Re-review verdict

Persona	Severity	Finding
Security Auditor	—	None. No new inputs, secrets, auth, or attack surface in the delta.
Regression Skeptic	—	Resolved. The prior P2 (terminal count stuck for targetTotal % 32 ∈ [1,8] vaults) is fixed by the new terminal-state branch. No new regression introduced.
Performance Critic	—	Delta adds one extra index-progress trigger at completion (negligible). The prior P3 yield-drain tradeoff is unchanged and acceptable.
Architecture Judge	P3	(unchanged from prior review) window.setTimeout is still not registered with the Component lifecycle (registerInterval), so it isn't cleared on unload — the callback is guarded by this.active so it's safe, just not clean. index-progress is still fired via this.core.events.trigger(...) directly, bypassing the typed trigger overloads on Datacore.
QA Lead	P3	(unchanged) No automated coverage for the yield/progress logic; the 321 passing tests are pre-existing and don't exercise this path. The manual test (1,751 files) now exercises the terminal branch, but the small-vault edge case (targetTotal % 32 ∈ [1,8]) still lacks an automated test.
Product Reliability	—	Pending setTimeout survives unload (≤16 ms, guarded by active → no-op). No observability gap.

Opened Issues

• [Auto-Detected] Startup progress counter can get stuck before targetTotal for small vaults #186 — Resolved and closed. The terminal-state branch in this commit addresses the finding; closing comment posted with the fix trace.

Summary

The follow-up commit is a minimal, correct fix that resolves the only substantive finding from the prior review (#186, P2). The terminal index-progress event now fires exactly once at completion (initialized is monotonic and single-threaded inside handleResult, so no double-fire or race). Remaining items are P3 tech-debt nits (lifecycle registration of the timeout, typed trigger overload, test coverage) that are non-blocking and suitable for optional follow-up. Safe to merge.

[bitskc]

🔍 [Architecture Judge] P3: window.setTimeout is not registered with the Component lifecycle (registerInterval), so it isn't cleared on unload. The callback is guarded by this.active so it's safe, just not clean. Consider this.registerInterval(fn, YIELD_DELAY_MS) or tracking the timer for cleanup in onunload.

[bitskc]

🔍 [Architecture Judge] P3: index-progress is fired via this.core.events.trigger(...) directly, bypassing the typed trigger overloads on Datacore (lines 292-300). Consider adding an index-progress overload so the event is type-checked at the call site.

[bitskc]

Local CI Results

PR: #184 | SHA: a4a990e4f614a71be434a213882a5cb9c101c98b | Ran: 2026-08-16 12:24:42 UTC

Check	Status	Duration
Install dependencies	✅	2s
Tests (jest)	✅	10s
Format check (prettier)	❌	8s
Type check (tsc --noEmit)	✅	3s
Build (tsc + api-extractor + esbuild)	✅	7s

Overall: ❌ 1 check failed (pre-existing, not introduced by this PR)

Failures

yarn run prettier --check src

[warn] src/index/import/canvas.ts
[warn] Code style issues found in the above file(s). Forgot to run Prettier?
error Command failed with exit code 1.

Analysis: The formatting failure is in src/index/import/canvas.ts, which is not modified by this PR. This PR only touches src/index/datacore.ts and src/ui/loading-boundary.tsx. The same failure was present at the previous SHA (937c039) and is a pre-existing issue on master, not a regression introduced by this PR.

Warnings

• ts-jest: TypeScript 5.8.3 is installed but ts-jest 27 only supports <5.0.0. Tests still pass; this is a version-compatibility warning only.
• Build (api-extractor): Numerous ae-missing-release-tag warnings on exported types in src/index/types/ (missing @alpha/@beta/@public/@internal tags) and one tsdoc-code-fence-opening-indent warning in src/index/types/json/markdown.ts. These are pre-existing API documentation warnings; the build completes successfully.
• Install: Unmet peer dependency warnings for react/react-dom (from react-select, @fortawesome/react-fontawesome) and esbuild@latest peer mismatch from esbuild-plugin-inline-worker. Pre-existing; install succeeds.