Two valid findings from an opposite-model (Codex) review of this branch:
- provider-request: drop bare `unknown`/`unrecognized` from the structured-output
rejection keywords. They false-positive on model-name errors ("unknown model",
"unrecognized model ID" from Ollama/Bedrock), triggering a wasted fallback
retry. The specific feature tokens + bare `schema` still cover real structured-
output rejections, so true-positive detection is unaffected.
- streaming: streamErrorMessage's OpenAI-style `{error:{...}}` branch now only
treats the payload as an error when it carries a message/type, so a stray empty
`error: {}` (or code-only object) in a normal chunk no longer aborts the stream.
The Anthropic `{type:'error'}` branch still always signals an error.
Declined: Codex's suggestion to convert streaming's HTTP>=400 to ProviderApiError
+ add fallback — the streaming path always sends `structured:false`, so there is
no structured request to fall back from.
Tests updated/added: model-name 400s do not retry; empty/code-only error objects
do not throw.
Change-Id: I413d633c8ce7036365f97e2d8e1287c111e0902f
Assessment-driven batch of 10 verified, low-risk improvements. All gates green
(typecheck, biome, obsidian lint + strict review, 28 test files, branch
coverage 100%, e2e gate).
Correctness:
- provider-request: decide structured-output fallback on HTTP status via a new
ProviderApiError (status+body) instead of pattern-matching the i18n-translated
message — previously only en/zh matched, so fr/de/es/ja/ko users hit silent
permanent failures when a provider rejected json_schema.
- streaming: surface in-stream provider error payloads ({type:'error'} /
{error:{}}) by throwing, instead of swallowing them and later misreporting a
transient overload/quota error as "non-JSON LLM output". Note: detection runs
outside the JSON.parse try/catch so the throw is not swallowed.
- main: throwIfCancelled before cacheManager.put so a cancelled job cannot
poison the cache.
- generation-job-manager: add cancelAll(); onunload now cancels in-flight jobs
(aborting streaming HTTP + SIGKILL-ing CLI children), not just queued waiters.
Onboarding / UX:
- settings: DEFAULT_SETTINGS.promptLanguage 'zh' -> 'auto' so new non-Chinese
users get source-language summaries by default (existing users unaffected).
- view/main/types: first-run "Set up AI provider" CTA in the empty state when no
credential is configured (PluginHost.openSettings + isCredentialConfigured).
- error-ui/generation-job-manager/types: new 'network' ErrorKind with an
actionable notice + Retry for offline/connection failures.
- i18n-strings: 4 new keys across all 7 locales (parity test enforced).
CI / docs:
- .c8rc.json + package.json: branch-coverage gate (check-coverage, branches=100,
degenerate metrics disabled) and preserve c8's exit code in the coverage script.
- ci.yml: run coverage gate and strict obsidian review in CI.
- README: fix Obsidian version badge 1.4.0 -> 1.8.7 (matches manifest).
- e2e product-shell DOM shim: add createSpan (was missing; real Obsidian has it).
Tests: cover the status-based fallback (incl. non-English locale), in-stream
error throwing, cancelAll, and network classification.
Change-Id: Ic619098aa7cdf3dc1c444be4bb8a445550eadf55
- npm run coverage runs the full suite under c8
- Wire inline source maps into the esbuild test bundles
- Redirect bundles into repo (.test-bundles) under coverage so c8
processes them; cleaned automatically after the run
- Fix esbuild->v8-to-istanbul source path remap (tests/coverage-sourcemap.js)
- .c8rc.json, .gitignore, dev-dep c8
Note: statement/function totals are degenerate under esbuild bundling;
branch coverage + HTML report are the reliable signals.
Change-Id: I880f5ee1e9a8e5368e27a8e274825f917d972269
Claude CLI 2.1.131+ rejects `--output-format stream-json` unless `--verbose`
is also passed. Without it the CLI exits 1 with "When using --print,
--output-format=stream-json requires --verbose", breaking the Claude Code
backend.
Adds the flag in src/cli.ts and locks both the flag and full args contract
for Claude + Codex backends with new tests:
- testClaudeCodeArgs: extended with mcp-config / disallowed-tools / chrome
/ slash-commands assertions and --verbose position check
- testClaudeCodeStreamJsonResilience: banner-mixed NDJSON, missing result,
empty stdout, multiple result events, content fallback
- testClaudeCodeVerboseRegressionCanary: simulates real CLI stderr on
missing --verbose and asserts error pipeline preserves exitCode + stderr
- testCodexErrorPath: codex non-zero exit propagates structured error
- testCodexArgsMinimalContract: deepStrictEqual lock on codex args 5-tuple
- testClaudeBackendSurfacesVerboseError: smoke flow surfaces the canary
- testCodexBackendPropagatesVersionFailure: version probe failure blocks
smoke instead of silently continuing
Also wires npm run lint:obsidian into CI alongside Biome.
Closes#3
Change-Id: I6dbaa1b55172eb0b5b773bd25d1741b0603a4e4e
Replaces the single long-Notice failure UX with a kind-aware dispatcher
so users get actionable buttons instead of a wall of text.
- CliProcessError extends Error with a typed CliErrorDetails payload
(reason / pid / elapsed / idle / bytes / tails / exitCode / signal /
timeoutMs / idleTimeoutMs). All five runCli failure paths now throw
the typed error; message format is preserved so existing classifier
regex and tests still match.
- classifyGenerationError short-circuits on details.reason for the
deterministic cases (wall/idle-timeout → timeout, spawn/startup →
config, streams-unavailable → unknown). exit-nonzero falls through
so stderr-derived auth/rate-limit hints still classify correctly.
Duck-typed check avoids cli ↔ generation-job-manager circular import.
- New error-ui.ts dispatches by ErrorKind:
timeout (with details) → TimeoutDiagnosticsModal showing cmd, pid,
elapsed, idle, bytes, redacted stderr/stdout tail, plus copy /
open-settings / close buttons.
timeout (no details, e.g. API streaming) → actionable Notice.
auth/config → Notice + "Open Settings" CTA.
rate-limit → Notice + "Copy details".
schema → Notice + "Copy raw output" CTA.
unknown → unchanged legacy short Notice.
- main.ts handleGenerationError delegates to dispatcher; new
openPluginSettings() helper uses Obsidian's app.setting.openTabById.
- 21 i18n strings (zh + en) for all new UI labels and the modal copy.
- styles.css adds error notice + diagnostics modal styling.
- 7 new test assertions cover typed details on every failure path and
classifyGenerationError respecting structured reasons.
Change-Id: I8af46d375d92ba26d14180ffbeb6c3a6dedd89f5
The CLI provider previously surfaced only "CLI timed out (Xms)" on hang,
discarding accumulated stderr/stdout, pid, and timing — leaving users
unable to tell network hangs from auth failures from a slow model.
- runCli timeout/idle errors now include pid, elapsed, idle duration,
utf-8 byte counts, plus stderr/stdout tails (with secret redaction
for Bearer tokens, sk-* keys, and Authorization-style headers).
- Add optional cliIdleTimeoutMs (default 0/disabled): kill the CLI if
no output for N ms, independent of the wall-clock cliTimeoutMs.
- Add optional debugLogging: console hooks at spawn/ok/fail with pid
and byte counts for live diagnosis.
- Settled-guard on noteActivity/armIdleTimer prevents stale timers
after first-wins settle (cancel / wall / idle / child-error / close).
- New unit coverage: rich diagnostic suffix, idle timeout firing, idle
reset on heartbeat, secret redaction, idle normalization edge cases.
Change-Id: I9191f8d348ef43ff9605dc5169f0951f5af85d26
Workspace.revealLeaf was added in Obsidian 1.7.2 but the plugin has
been declaring minAppVersion 1.4.0 since 1.0.0. Update the manifest
and pin 1.0.15 in versions.json so the install gate reflects reality.
Older release entries in versions.json remain at 1.4.0 to avoid
retroactively changing their published compat declaration.
Change-Id: I1fa8c02770b4e88f9f1e1b31835adf4d4af5d718
Add eslint v9 + @typescript-eslint/parser + eslint-plugin-obsidianmd as
a dedicated `npm run lint:obsidian` track, separate from the existing
biome lint. Apply autofixes from the recommended config and resolve the
follow-on type/test issues.
- prefer-create-el: createEl('div'/'span') -> createDiv/createSpan
- prefer-active-window-timers: setTimeout/clearTimeout -> activeWindow.*
+ retype timer holders from ReturnType<typeof setTimeout> to number
- prefer-active-doc: drop globalThis.fetch in streaming
- no-useless-catch: drop no-op rethrow wrapper
- no-restricted-globals: streaming.ts is exempted because requestUrl
does not support streaming responses
- @typescript-eslint/no-unsafe-*: disabled (out of scope for an Obsidian
plugin lint pass; tsc + biome already cover type safety)
- tests: polyfill globalThis.activeWindow so unit tests still run in Node
Change-Id: I43cc652e29a66d490e2834e940843c39b211b80b
The settings tab listed ~23 entries on first paint (API mode), most of
which the typical user never touches. Restructured into 5 sections,
of which only the first two are expanded by default.
Sections (default visibility):
- Quick setup (open): backend / provider preset / credential / model / test
- Reading output (open): prompt language / card range / export folder / UI lang
- Advanced connection: api format / base URL / auth type / headers /
max tokens / streaming + timeout merged
- Advanced prompt: max input chars / custom system prompt
- Cache & maintenance: max entries / clear-all
First-paint count drops from ~23 to ~9 rows.
Smart auto-expand: Advanced connection opens automatically when the
user has departed from preset defaults — custom-* provider, custom
headers, non-default base URL, non-auto auth type, streaming=false,
non-default max tokens, or api format mismatching preset. Same idea
for Advanced prompt (custom prompt or non-default max input chars)
and CLI advanced (non-default cli timeout). Old users find their
configured values without manual digging.
Other simplifications:
- API key + env var fallback combined into one row. The env var input
lives behind a tiny details/summary disclosure and is built with
raw DOM (label + input) instead of a nested `new Setting` to avoid
Obsidian theme CSS conflicts on .setting-item.
- Provider preset row gains an inline read-only summary of the
derived API format and base URL, replacing the standalone rows
for those two fields under built-in presets.
- Streaming toggle and streaming timeout are now one row; the
timeout input is wrapped in its own div and toggled visible only
when streaming is on.
No data.json changes — every settings key keeps its existing name and
default value. Existing installs upgrade with zero migration.
styles.css: add scoped classes for the collapsible sections, env-var
inline row, timeout inline input, and a generic .parallel-reader-hidden.
i18n: 7 new keys × zh+en for section headings + summary fallbacks.
Codex review pass:
- shouldOpenAdvancedConnection now also reacts to streaming=false /
non-default apiMaxTokens / apiFormat ≠ preset.format.
- streaming + timeout merged row hides a dedicated wrapper instead of
inputEl.parentElement (which on some themes shares the controlEl
with the toggle).
- env-var fallback no longer nests `new Setting` inside a controlEl.
- "(no base url)" placeholder in the preset summary is now i18n'd.
Change-Id: I8ef11c56a89121df13a630b1c724652e78b4069f
UX change:
- The "open Parallel Reader" command now toggles the right sidebar's
collapsed state instead of detaching the leaf. The tab and its content
are preserved across hide/show, matching standard Obsidian behaviour
for File Explorer and friends. Detaching the tab on every second
press destroyed cached cards and forced re-generation.
Behaviour:
- No leaf yet → create + reveal
- Leaf exists, right sidebar collapsed → reveal (expands sidebar)
- Leaf exists, right sidebar expanded → collapse(); tab is preserved
- No right sidebar (mobile / unusual layout) → reveal as fallback
Tests:
- New tests/view-render.test.js (18 cases) covering the component layer
that was previously a gap. The 1.0.12 → 1.0.13 sidebar regression
could not have been caught by any existing pure-function test; this
file fills that gap by mocking app.workspace + view spies and running
real runForFile / refreshViewAfterCache* / toggleParallelView.
- shouldRender matrix (4 cases): silentView × sourceFile state — the
exact regression introduced in PR2 where guards were over-applied.
- toggleParallelView (4 cases): no-leaf opens, expanded → collapse,
collapsed → reveal, no-rightSplit fallback.
- refreshViewAfterCacheDelete + Clear (4 cases): matching file vs
different file vs no-view, plus clear-all always renderEmpty.
- runForFile outcome enum mapping (6 cases): already-running early
return, already-running from catch, cancelled, generic error,
regenerate-confirm cancellation, skipEditConfirm bypass — these
drive batch statistics and were untested before.
- view.deleteCard / updateCard cardPersistFailed path (3 cases):
exposes ParallelReaderView via test-exports for instantiation;
mocks cacheReplaceCards returning false to verify the failure
notice path lands and the public method returns false.
src/test-exports.ts: export ParallelReaderView for component tests.
tests/catalog.json: register view-render.test.js under component.
Change-Id: I439b5fca07b0928e6fe44d9025c2421c71c6cb28
Two regressions reported after 1.0.12 was installed:
1. Hotkey-triggered Generate did not update the sidebar. PR2 added
`viewIsShowingFile(view, file)` guards to renderLoading/loadFor/
renderError in runForFile to keep batch (silentView) jobs from
disturbing a panel showing a different note. The guard was applied
to all paths, breaking the interactive case: when the sidebar had
never been opened (sourceFile=null) or was showing a different note,
every render call was skipped and the sidebar stayed blank while
generation completed in the background.
Fix: compute `shouldRender = !silentView || viewIsShowingFile(...)`
once. Non-silent (user pressed Generate on this file) renders
unconditionally; silentView keeps the existing "only if it's already
this file" behaviour.
2. The open-view command had no toggle behaviour. Pressing the hotkey a
second time silently re-opened the same panel.
Fix: if a parallel-reader leaf already exists, detach it; otherwise
ensureView + sync.
Change-Id: I1db3d70806d8d07965d18bf3717023de027ac577
Findings from the original review (P2):
- streaming: flush unterminated final SSE event at EOF (some providers
close the stream without a trailing blank line, dropping the last delta)
- cache-manager: validate entry shape on load — drop entries where
cards is not an array, anchor is not a string, or bullets is not an
array. Tolerates missing optional fields (treated as cache miss).
- view: card edit/delete now check cacheReplaceCards return and surface
failures via a localized Notice instead of pretending success
- main: clear-current / clear-all / file-menu-clear refresh open view
via renderEmpty so stale UI does not display deleted data
- prompt + settings-tab: card count is normalized via a single helper
used by buildPrompts and onChange. Prompt and fingerprint stay in sync.
UI value is written back to the textbox after clamping.
- generation-job-manager: global concurrency limit (default 3) with a
cancellable wait queue. Race-safe slot accounting via a reserved
counter so resolved-but-not-yet-set waiters are visible to fast-path
start() callers.
- runForFile: now returns RunForFileResult; accepts preloadedContent +
silentView + skipEditConfirm options used by batch (and reflected on
the PluginHost interface).
- batch: avoid double file read, do not steal UI focus, classify
results correctly (generated / cached / already-running / empty / ...)
Follow-up from the 1.0.11 review:
- generationFingerprint: codex backend excludes settings.model from the
hash. Codex ignores --model; previously editing model spuriously
invalidated all codex cache.
Codex review pass:
- generation-job-manager: race fix — releaseSlot's resolve microtask
and a synchronous start() fast-path could briefly overshoot
maxConcurrent. Reserve the slot synchronously inside the wrapped
resolve to close the window.
- cache-manager: anchor type validation in addition to bullets.
NOTE: Codex backend users will see a one-time "stale cache" banner on
existing notes due to the fingerprint change; regenerate to refresh.
Change-Id: I7721c7dfe51dea3f51b0215764f721523c2f6806
Drop the self-contained/no-Python historical note and the contributor-
focused tests/catalog.json paragraph; keep the gate command + TEST_LIVE=1
hint. Public README does not need internal test classification details.
Change-Id: Ic7a2ed45af0ebeb3be227aed03079ce258ed30c2
P1:
- cli: pin codex --sandbox read-only (defeat prompt injection)
- settings: clear apiKey/apiHeaders on provider preset switch
- main/view: render error state when LLM returns empty cards
- schema/cli: do not log raw model output; UI errors expose length only
P2:
- cli: pass --model to Claude Code when settings.model set
- settings-tab: add CLI timeout input + normalizeCliTimeoutMs (min 1s)
- view: export source link uses [[path|basename]]
- view: export failures surface a localized Notice
P3:
- schema: cardUntitled fallback now respects uiLanguage
- generation-job-manager: error codes localized via main.ts
- batch: settled guard on promptForBatchFolder modal
- README: drop stale e2e validator install note
Codex review pass:
- provider-parsers: forward settings to normalizeCardsPayload
- generation-job-manager: classifyGenerationError regex covers new EN/zh
schema-error messages
- cli: do NOT pass --model to codex (DEFAULT_SETTINGS.model is a Claude
name and would break codex; codex relies on its own config)
Tests: cli (codex sandbox + claude --model + classify zh/en), settings
(applyApiProviderPreset isolation + normalizeCliTimeoutMs), schema
(language-aware fallback title), test-exports (new symbols).
Change-Id: I9f4c21f9299e1a9bf166f69f712859854482fe92
- bump-version.mjs: reject duplicate versions (use --force to override),
auto git-add manifest.json + versions.json for npm version hook
- run-tests.mjs: use execFileSync to handle paths with spaces
- test-setup.js: clean up esbuild temp directory on process exit
Change-Id: I44410b550d09487dd7f5e30a8f9385a2c500252a
