feat(chunker): add .svelte support with two-phase TypeScript injection#128
Open
AutumnsGrove wants to merge 5 commits intoory:mainfrom
Open
feat(chunker): add .svelte support with two-phase TypeScript injection#128AutumnsGrove wants to merge 5 commits intoory:mainfrom
AutumnsGrove wants to merge 5 commits intoory:mainfrom
Conversation
Member
|
I see this has a new chunker for svelte - in that case I think we need a bench-swe suite that has an appropriate test. It sometimes takes a few attempts to find a good test case, because not all test cases can be solved well by claude. If lumen is not performing, then there can be multiple reasons:
You can use claude to analyze the raw json files. It takes a few attempts to get this working well, but once it does, it actually helps a lot! |
Contributor
Author
|
@aeneasr I can do that. I just added the basic svelte support - I wasn't aware of your swe bench suites. I'll take a look at it and add to this PR. The issue I experienced is svelte wasn't indexed at all - this Pr attempts to implement that. When I tested it locally via my built mcp server, it properly picked up the svelte files and worked flawlessly. I tried to build on your existing parsing tooling. |
Contributor
Author
|
@aeneasr
One thing to flag — TestLang_Python/HTTP_route_handler_decorator is failing locally with a snapshot drift |
Member
|
7979d7f is passing so the snapshot drift should be from your work, not broken on master The Kotlin PR is pretty messy still. |
Member
|
Could you please run the benchmark also? :) So we know if Lumen is properly indexing svelte! And commit the result benchmark (like we have for the other languages) |
Contributor
Author
Absolutely. Ill run this tonight. @aeneasr |
📝 WalkthroughWalkthroughThis PR extends the codebase to support Svelte files by implementing a dedicated Svelte chunker that parses embedded TypeScript script blocks, adds comprehensive test fixtures and benchmark tasks, and updates test expectations to account for indexed Svelte files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Index .svelte files by parsing the outer Svelte grammar to locate
<script> blocks, then re-parsing each block's raw_text with the
TypeScript chunker to extract named symbols (functions, classes,
interfaces, etc.). Line numbers are adjusted to be file-relative so
search results point to the correct lines in the original .svelte file.
Template syntax ({#if}, {#each}, bind:) and Svelte rune calls
($state(), $derived()) are handled transparently — runes parse as
ordinary TypeScript call-expression initializers.
Also registers text-embedding-voyage-4-nano in KnownModels (LM Studio,
1024 dims, 2048 ctx).
Fixes three hook tests that failed when a user config file at
~/.config/lumen/config.yaml set a non-default embedding model: the
tests now set XDG_CONFIG_HOME to a temp dir so both writeHookTestDB
and the hook use the same default model, preventing DB-path hash
mismatches and dimension-mismatch schema resets.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This model is LM Studio-only (served via Voyage AI's local inference) and not available in Ollama. The project defaults to Ollama and runs e2e tests against it, so registering an LM Studio-exclusive model here is misleading. Users who want voyage-4-nano can still configure it manually via LUMEN_EMBED_MODEL — it just won't have pre-registered dims/ctx/min-score. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Vendor 9 real-world .svelte components from huggingface/chat-ui (Apache 2.0, commit c0cfbdf) into testdata/fixtures/svelte/ - Add testdata/sample-project/Dashboard.svelte as the E2E fixture component with ActivityCache class, loadUserActivity, and handleRefresh symbols for semantic search verification - Add TestE2E_SvelteIndexing: asserts .svelte files are indexed, file-relative line numbers are correct, and symbols from the script block surface in semantic_search results - Update all hardcoded file-count assertions (5 → 6, and 6 → 7 for the incremental test) to account for Dashboard.svelte - Isolate E2E subprocess config via XDG_CONFIG_HOME so tests are hermetic regardless of local ~/.config/lumen/config.yaml - Add all-minilm model preflight check in TestMain with a clear error message if the model is not installed in Ollama Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace hand-authored Dashboard.svelte with mcp-server-card.svelte from huggingface/chat-ui (already vendored in testdata/fixtures/svelte/). All sample project files now originate from established open-source repos. Update TestE2E_SvelteIndexing to search for symbols from the real file (setEnabled, handleHealthCheck, handleDelete). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add SWE-bench benchmark for Svelte using huggingface/chat-ui#2033 (dark theme close button hover bug). Results show -26% cost and -31% time with Lumen vs baseline, consistent with other languages. - Add bench-swe/tasks/svelte/hard.json and gold patch - Commit benchmark results (Haiku, jina-embeddings-v2-base-code) - Update README with Svelte in all benchmark tables (8→9 languages) - Fix runner to fall back to full clone when GitHub rejects direct SHA fetch (needed for huggingface/chat-ui) 🤖 Generated with [Claude Code](https://claude.ai/code) Model: claude-opus-4-6 Co-Authored-By: Claude <noreply@anthropic.com>
AutumnsGrove
force-pushed
the
feat/svelte-support
branch
from
cdff9f4 to
e52e61f
Compare
Contributor
Author
|
@aeneasr Results (ordis/jina-embeddings-v2-base-code, Haiku): -26% cost, -31% time, -26% tokens — consistent with the other languages. Both approaches got Poor (Haiku couldn't nail the exact Tailwind dark mode classes), but Lumen found the right area faster. Also had to add a full-clone fallback to the bench-swe runner — GitHub doesn't support allowReachableSHA1InWant, so git fetch --depth=1 origin fails for repos like chat-ui. The fallback only triggers when the direct SHA fetch is rejected. Snapshot drift on TestLang_Python/HTTP_route_handler_decorator is clean now after rebasing onto current main. Full results in bench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (1)
internal/chunker/svelte_test.go (1)
82-111: Refactor repeated no-script cases into a table-driven test.
TestSvelteChunker_EmptyScriptandTestSvelteChunker_NoScriptare parallel variants and fit a single table-driven test for maintainability and consistency.Proposed refactor
-func TestSvelteChunker_EmptyScript(t *testing.T) { - empty := []byte(`<script> -</script> -<p>hello</p> -`) - langs := chunker.DefaultLanguages(512) - c := langs[".svelte"] - chunks, err := c.Chunk("empty.svelte", empty) - if err != nil { - t.Fatalf("Chunk: %v", err) - } - if len(chunks) != 0 { - t.Errorf("expected 0 chunks for empty script, got %d", len(chunks)) - } -} - -func TestSvelteChunker_NoScript(t *testing.T) { - noScript := []byte(`<h1>Static page</h1> -<p>No script block here.</p> -`) - langs := chunker.DefaultLanguages(512) - c := langs[".svelte"] - chunks, err := c.Chunk("static.svelte", noScript) - if err != nil { - t.Fatalf("Chunk: %v", err) - } - if len(chunks) != 0 { - t.Errorf("expected 0 chunks for file with no script, got %d", len(chunks)) - } -} +func TestSvelteChunker_NoSymbolsCases(t *testing.T) { + tests := []struct { + name string + path string + content []byte + }{ + { + name: "empty script", + path: "empty.svelte", + content: []byte(`<script> +</script> +<p>hello</p> +`), + }, + { + name: "no script", + path: "static.svelte", + content: []byte(`<h1>Static page</h1> +<p>No script block here.</p> +`), + }, + } + + langs := chunker.DefaultLanguages(512) + c, ok := langs[".svelte"] + if !ok { + t.Fatal("no chunker registered for .svelte") + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + chunks, err := c.Chunk(tc.path, tc.content) + if err != nil { + t.Fatalf("Chunk: %v", err) + } + if len(chunks) != 0 { + t.Errorf("expected 0 chunks, got %d", len(chunks)) + } + }) + } +}As per coding guidelines: "Use table-driven tests for multiple test cases in Go".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/chunker/svelte_test.go` around lines 82 - 111, Combine the two parallel tests TestSvelteChunker_EmptyScript and TestSvelteChunker_NoScript into a single table-driven test that iterates cases with a name and input bytes, obtains the Svelte chunker via chunker.DefaultLanguages(512)[".svelte"], calls c.Chunk(fileName, data) for each case, and asserts err==nil and len(chunks)==0 using t.Run for each named case; this removes duplicated setup and keeps behavior identical to the existing tests (use the same file names "empty.svelte"/"static.svelte" or derive them from the table entries).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@bench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/detail-report.md`:
- Line 213: The markdown contains unlabeled fenced code blocks around the
snippet including the token "peer": true which triggers MD040; update those
triple-backtick fences to include a language token (for example change ``` to
```text or ```diff) for each unlabeled block (both occurrences referenced in the
review) so markdownlint passes; ensure you edit the fenced code blocks that wrap
the "peer": true snippet and the other unlabeled block mentioned so both have a
language label.
In
`@bench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-baseline-judge.md`:
- Around line 1-3: Update the judge evaluation text to correct the factual
error: state that the gold patch contains only one fix (the modal close button
for the "Pasted Content" panel) which adds the classes "dark:text-gray-400
dark:hover:text-white", and remove the claim that the gold patch also added an
image close button fix ("dark:bg-gray-700"); keep mention that the candidate
patch changed package-lock.json peer dependency flags and only applied the image
close button change while missing the modal close-button fix in
UploadedFile.svelte.
In `@bench-swe/internal/runner/runner.go`:
- Around line 75-89: The fetch+fallback share cloneCtx so a slow initial
exec.CommandContext call can leave the fallback using an almost-expired context;
change the fallback path to (1) detect if the first fetch error was a context
cancellation/deadline (errors.Is(err, context.Canceled) || errors.Is(err,
context.DeadlineExceeded)) and if so return that original fetch error instead of
trying a retry, and (2) when proceeding with the clone fallback, create a fresh
context with an appropriate timeout (e.g. via context.WithTimeout) and use that
new context for exec.CommandContext for the "git clone" and subsequent "git -C
... checkout" commands (refer to shallowArgs, cloneCtx, and the
cloneArgs/checkout exec.CommandContext calls).
In `@bench-swe/tasks/svelte/hard.json`:
- Around line 12-13: The benchmark's setup_commands are incomplete: update the
setup sequence referenced by the "setup_commands" entry so it installs
dependencies, generates SvelteKit config, and installs Playwright browsers
before running tests (e.g., run npm install, then npm run prepare or npx
svelte-kit sync to create .svelte-kit/tsconfig.json, and npx playwright install
to fetch browsers); leave the "test_command" as "npx vitest run". Ensure these
steps are run in that order so SvelteKit base config and Playwright browser
executables exist before Vitest executes.
In `@internal/chunker/svelte_test.go`:
- Around line 77-79: The test dereferences chunks[0] without checking length;
modify the test in internal/chunker/svelte_test.go to first assert len(chunks) >
0 (use t.Fatalf or t.Errorf with a clear message like "expected at least one
chunk, got 0") and return on failure, then perform the existing StartLine
assertion on chunks[0]; this prevents a panic if symbol extraction yields no
chunks while preserving the original check on chunks[0].StartLine.
In `@internal/chunker/svelte.go`:
- Around line 49-53: chunkScriptElement calls s.tsChunker.Chunk unconditionally
which can panic if the injected tsChunker is nil; update the SvelteChunker
methods (e.g., Chunk and any other places that call chunkScriptElement or
directly call s.tsChunker.Chunk) to check for nil s.tsChunker before invoking
Chunk and return a clear error (e.g., fmt.Errorf("tsChunker is nil") or wrap
with file context) or handle fallback behavior, and apply the same nil-guard in
the other location that calls s.tsChunker (the block referenced around lines
89-92) so all uses of s.tsChunker are protected from nil dereference.
- Around line 59-62: The code silently ignores errors from s.chunkScriptElement
when iterating script nodes; change the logic in the enclosing function (the
caller that currently does sc, err := s.chunkScriptElement(filePath, content,
node)) to handle non-nil err instead of dropping it—either return the error
immediately, aggregate it into an error slice and return a combined error after
processing, or log it with context before proceeding; ensure you include the
filePath and node context in the reported error and keep the successful sc
appended to chunks when err is nil so partial successes remain intact.
- Around line 25-49: The repository's index format must be bumped to reflect the
new SvelteChunker: update the IndexVersion constant in
internal/config/version.go (the symbol IndexVersion) by incrementing its value
(currently "2") to the next integer (e.g., "3") so existing indexes are
invalidated; change the constant, run the test suite/build to ensure no other
version-dependent code needs adjustment, and commit the version change alongside
the SvelteChunker addition.
In `@testdata/fixtures/svelte/add-server-form.svelte`:
- Line 224: Update the security warning text string "Never share confidental
informations." to the corrected phrase "Never share confidential information."
Locate the literal text in the Svelte template (search for the exact incorrect
string) and replace it, preserving surrounding markup and spacing; ensure only
the typo is fixed and no other text or element structure is changed.
- Around line 48-51: The removeHeader function deletes a header row but only
deletes showHeaderValues[index], leaving other numeric keys mismatched after
headers array indices shift; update removeHeader (referencing function
removeHeader, variables headers and showHeaderValues) to rebuild or reindex
showHeaderValues after filtering headers so keys match new indices — e.g. create
a new object and copy existing visibility values for each remaining header by
their new index (or iterate headers and set newShowHeaderValues[i] =
showHeaderValues[oldIndex]) then assign showHeaderValues = newShowHeaderValues.
In `@testdata/fixtures/svelte/audio-player.svelte`:
- Around line 63-84: The progress bar math can produce NaN/Infinity when
duration is 0 or non-finite; update the conditional and width calculation around
the slider so it only renders / computes when duration is a positive finite
number (e.g., replace the {`#if` duration !== Infinity} check with a guard like
duration > 0 && isFinite(duration)) and ensure the inline style for width
(currently "width: {(time / duration) * 100}%") uses a safe fallback (0%) when
duration is zero/non-finite; also adjust related aria-valuemax/aria-valuenow
uses of duration/time in the same block to avoid invalid accessibility values
(affecting the div role="slider", the style width, and aria-valuemax variables
such as duration, and the seek/onpointer handlers).
- Around line 66-78: The custom slider div with role="slider" (using
aria-valuenow={time}, aria-valuemin/min, tabindex="0") is not keyboard-operable;
add keyboard handling to the element (e.g., an onkeydown handler on the same
div) in the audio-player component to respond to ArrowLeft/ArrowRight (small
step), PageUp/PageDown (larger step), Home/End (min/max) keys, update the time
value and aria-valuenow accordingly, call the existing seek(time) function to
apply the change, preventDefault for handled keys, and ensure interaction with
the paused variable and existing onpointerdown/onpointerup behavior remains
consistent.
In `@testdata/fixtures/svelte/markdown-renderer.svelte`:
- Around line 43-57: The render can get stuck because the async IIFE that calls
processBlocks and the worker message path lack error handling and never call
updateDebouncer.endRender() on failure; wrap the async IIFE body that calls
processBlocks(content, sources) and handleBlocks(...) in try/catch/finally and
call updateDebouncer.endRender() in the finally block (ensure errors are logged
or forwarded), and add robust error handling for the worker path by adding
worker.onerror and/or wrapping worker.onmessage handler logic (the code that
inspects data and calls handleBlocks) in try/catch so that on exceptions you
call updateDebouncer.endRender() and log the error; reference processBlocks,
handleBlocks, updateDebouncer.startRender()/endRender(), MarkdownWorker, and
worker.onmessage when making these changes.
In `@testdata/fixtures/svelte/voice-recorder.svelte`:
- Around line 49-101: The async startRecording function can finish after the
component is unmounted and then initialize audio resources; add a
mounted/aborted guard immediately before and after awaiting
navigator.mediaDevices.getUserMedia to ensure the component is still live — if
the guard shows unmounted, stop and release the obtained stream (stop its
tracks), do not assign mediaStream, do not create audioContext or mediaRecorder,
and return early; apply the same mount-check/early-abort pattern to the other
async block mentioned (lines 160-167) so functions that await getUserMedia or
other async setup verify the component is still mounted before creating or
assigning media resources (mediaStream, audioContext, mediaRecorder,
startVisualization).
- Around line 103-139: stopRecording() can be called multiple times causing
races by overwriting mediaRecorder.onstop and calling mediaRecorder.stop()
repeatedly; fix by making stopRecording idempotent: add an internal single
shared stopping promise/flag (e.g., stopPromise or isStopping) inside the module
so subsequent calls return the same Promise and early-return if stopping is in
progress, ensure you only set mediaRecorder.onstop once and call
mediaRecorder.stop() only when not already stopping, and keep existing cleanup
steps (stopVisualization, mediaStream tracks stop, audioContext.close, analyser
nulling, mediaRecorder nulling, resolving with Blob from audioChunks) but funnel
all callers through that single in-flight stop promise for stopRecording, and
apply the same pattern to the other stop routine at lines 142-167.
---
Nitpick comments:
In `@internal/chunker/svelte_test.go`:
- Around line 82-111: Combine the two parallel tests
TestSvelteChunker_EmptyScript and TestSvelteChunker_NoScript into a single
table-driven test that iterates cases with a name and input bytes, obtains the
Svelte chunker via chunker.DefaultLanguages(512)[".svelte"], calls
c.Chunk(fileName, data) for each case, and asserts err==nil and len(chunks)==0
using t.Run for each named case; this removes duplicated setup and keeps
behavior identical to the existing tests (use the same file names
"empty.svelte"/"static.svelte" or derive them from the table entries).
🪄 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: 6c1b7303-b334-4c09-a8f4-fe6cfeb57ab2
⛔ Files ignored due to path filters (3)
bench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-baseline-stderr.logis excluded by!**/*.logbench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-with-lumen-stderr.logis excluded by!**/*.loggo.sumis excluded by!**/*.sum
📒 Files selected for processing (38)
README.mdbench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/detail-report.mdbench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/summary-report.mdbench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-baseline-judge.jsonbench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-baseline-judge.mdbench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-baseline-metrics.jsonbench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-baseline-patch.diffbench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-baseline-raw.jsonlbench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-baseline-tests.txtbench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-with-lumen-judge.jsonbench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-with-lumen-judge.mdbench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-with-lumen-metrics.jsonbench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-with-lumen-patch.diffbench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-with-lumen-raw.jsonlbench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-with-lumen-tests.txtbench-swe/internal/runner/runner.gobench-swe/patches/svelte-hard.patchbench-swe/tasks/svelte/hard.jsoncmd/hook_test.goe2e_cli_test.goe2e_test.gogo.modinternal/chunker/languages.gointernal/chunker/svelte.gointernal/chunker/svelte_test.gointernal/chunker/treesitter_test.gointernal/models/models.gotestdata/fixtures/SOURCES.mdtestdata/fixtures/svelte/add-server-form.sveltetestdata/fixtures/svelte/audio-player.sveltetestdata/fixtures/svelte/audio-waveform.sveltetestdata/fixtures/svelte/markdown-renderer.sveltetestdata/fixtures/svelte/mcp-server-card.sveltetestdata/fixtures/svelte/mcp-server-manager.sveltetestdata/fixtures/svelte/modal.sveltetestdata/fixtures/svelte/nav-conversation-item.sveltetestdata/fixtures/svelte/voice-recorder.sveltetestdata/sample-project/mcp-server-card.svelte
| "integrity": "sha512-cJW4Xd/G3v5ovXtJJ52MAOclqeac9S/aGGgRzLabuF8TnIb6xHvMzKIa6JmrRzUkeXJgfL1MhukP0NK6l39h3A==", | ||
| "devOptional": true, | ||
| "license": "Apache-2.0", | ||
| - "peer": true, |
There was a problem hiding this comment.
Specify fence languages for markdownlint compliance.
Line 213 and Line 355 use unlabeled fenced code blocks (MD040). Add a language token (e.g., text or diff) to keep docs lint-clean.
📝 Suggested change pattern
-```
+```text
...
-```
+```Also applies to: 355-355
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 213-213: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@bench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/detail-report.md`
at line 213, The markdown contains unlabeled fenced code blocks around the
snippet including the token "peer": true which triggers MD040; update those
triple-backtick fences to include a language token (for example change ``` to
```text or ```diff) for each unlabeled block (both occurrences referenced in the
review) so markdownlint passes; ensure you edit the fenced code blocks that wrap
the "peer": true snippet and the other unlabeled block mentioned so both have a
language label.
Comment on lines
+1
to
+3
| ## Rating: Poor | ||
|
|
||
| The candidate patch makes changes to `package-lock.json` (peer dependency flags) and partially addresses the dark theme button visibility issue in `UploadedFile.svelte`, but misses the critical fix in the close button for the `Pasted Content` panel modal (around line 95). The gold patch fixes two locations: the modal close button (line 95) adding `dark:text-gray-400 dark:hover:text-white`, and the image close button (line 238) adding `dark:bg-gray-700`. The candidate only fixes the image close button at line 238 but omits the modal close button fix at line 95, leaving the original bug partially unresolved. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the gold patch includes both button fixes mentioned in the judge evaluation
echo "=== Checking gold patch content ==="
cat bench-swe/patches/svelte-hard.patch
echo -e "\n=== Searching for line 238 references ==="
rg -n "238" bench-swe/patches/svelte-hard.patch || echo "No line 238 reference found"
echo -e "\n=== Counting diff hunks ==="
rg "^@@" bench-swe/patches/svelte-hard.patch | wc -lRepository: ory/lumen
Length of output: 764
Correct the judge evaluation: the gold patch contains only ONE fix, not two.
The gold patch at bench-swe/patches/svelte-hard.patch contains only the modal close button fix at line 95 (dark:text-gray-400 dark:hover:text-white). The claimed image close button fix at line 238 adding dark:bg-gray-700 does not exist in the patch. The judge description is incorrect when it states "The gold patch fixes two locations." It fixes only the modal close button; the image close button fix is absent from the gold patch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@bench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-baseline-judge.md`
around lines 1 - 3, Update the judge evaluation text to correct the factual
error: state that the gold patch contains only one fix (the modal close button
for the "Pasted Content" panel) which adds the classes "dark:text-gray-400
dark:hover:text-white", and remove the claim that the gold patch also added an
image close button fix ("dark:bg-gray-700"); keep mention that the candidate
patch changed package-lock.json peer dependency flags and only applied the image
close button change while missing the modal close-button fix in
UploadedFile.svelte.
Comment on lines
+75
to
+89
| shallowArgs := []string{"-C", repoDir, "fetch", "--quiet", "--depth=1", "--filter=blob:none", "origin", t.BaseCommit} | ||
| if _, err := exec.CommandContext(cloneCtx, "git", shallowArgs...).CombinedOutput(); err != nil { | ||
| // Direct SHA fetch failed — remove partial repo and do a full clone instead. | ||
| // Partial clone config left behind by --filter=blob:none would cause checkout | ||
| // to lazily fetch objects by SHA, which GitHub also rejects. | ||
| fmt.Printf(" %-20s direct SHA fetch failed, falling back to full clone\n", t.ID) | ||
| _ = os.RemoveAll(repoDir) | ||
| cloneArgs := []string{"clone", "--quiet", t.Repo, repoDir} | ||
| if out, err := exec.CommandContext(cloneCtx, "git", cloneArgs...).CombinedOutput(); err != nil { | ||
| return nil, fmt.Errorf("git clone fallback: %w\n%s", err, out) | ||
| } | ||
| } | ||
|
|
||
| if out, err := exec.CommandContext(cloneCtx, "git", "-C", repoDir, "checkout", "--quiet", t.BaseCommit).CombinedOutput(); err != nil { | ||
| return nil, fmt.Errorf("git checkout: %w\n%s", err, out) |
There was a problem hiding this comment.
Reset the timeout for the fallback clone path.
The direct fetch, fallback clone, and final checkout all share cloneCtx. If the first fetch fails late, the fallback inherits an almost-expired context and can fail immediately even though the full clone would otherwise succeed. Since runner.Run has no outer retry, this turns the fallback into a flaky hard failure. Give the fallback path its own timeout, and return the original fetch error when the first attempt died due to timeout/cancellation rather than retrying with the same context.
Suggested fix
- shallowArgs := []string{"-C", repoDir, "fetch", "--quiet", "--depth=1", "--filter=blob:none", "origin", t.BaseCommit}
- if _, err := exec.CommandContext(cloneCtx, "git", shallowArgs...).CombinedOutput(); err != nil {
+ shallowArgs := []string{"-C", repoDir, "fetch", "--quiet", "--depth=1", "--filter=blob:none", "origin", t.BaseCommit}
+ if out, err := exec.CommandContext(cloneCtx, "git", shallowArgs...).CombinedOutput(); err != nil {
+ if cloneCtx.Err() != nil {
+ return nil, fmt.Errorf("git fetch base commit: %w\n%s", err, out)
+ }
+
// Direct SHA fetch failed — remove partial repo and do a full clone instead.
// Partial clone config left behind by --filter=blob:none would cause checkout
// to lazily fetch objects by SHA, which GitHub also rejects.
fmt.Printf(" %-20s direct SHA fetch failed, falling back to full clone\n", t.ID)
_ = os.RemoveAll(repoDir)
+
+ fallbackCtx, fallbackCancel := context.WithTimeout(ctx, 10*time.Minute)
+ defer fallbackCancel()
cloneArgs := []string{"clone", "--quiet", t.Repo, repoDir}
- if out, err := exec.CommandContext(cloneCtx, "git", cloneArgs...).CombinedOutput(); err != nil {
+ if out, err := exec.CommandContext(fallbackCtx, "git", cloneArgs...).CombinedOutput(); err != nil {
return nil, fmt.Errorf("git clone fallback: %w\n%s", err, out)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| shallowArgs := []string{"-C", repoDir, "fetch", "--quiet", "--depth=1", "--filter=blob:none", "origin", t.BaseCommit} | |
| if _, err := exec.CommandContext(cloneCtx, "git", shallowArgs...).CombinedOutput(); err != nil { | |
| // Direct SHA fetch failed — remove partial repo and do a full clone instead. | |
| // Partial clone config left behind by --filter=blob:none would cause checkout | |
| // to lazily fetch objects by SHA, which GitHub also rejects. | |
| fmt.Printf(" %-20s direct SHA fetch failed, falling back to full clone\n", t.ID) | |
| _ = os.RemoveAll(repoDir) | |
| cloneArgs := []string{"clone", "--quiet", t.Repo, repoDir} | |
| if out, err := exec.CommandContext(cloneCtx, "git", cloneArgs...).CombinedOutput(); err != nil { | |
| return nil, fmt.Errorf("git clone fallback: %w\n%s", err, out) | |
| } | |
| } | |
| if out, err := exec.CommandContext(cloneCtx, "git", "-C", repoDir, "checkout", "--quiet", t.BaseCommit).CombinedOutput(); err != nil { | |
| return nil, fmt.Errorf("git checkout: %w\n%s", err, out) | |
| shallowArgs := []string{"-C", repoDir, "fetch", "--quiet", "--depth=1", "--filter=blob:none", "origin", t.BaseCommit} | |
| if out, err := exec.CommandContext(cloneCtx, "git", shallowArgs...).CombinedOutput(); err != nil { | |
| if cloneCtx.Err() != nil { | |
| return nil, fmt.Errorf("git fetch base commit: %w\n%s", err, out) | |
| } | |
| // Direct SHA fetch failed — remove partial repo and do a full clone instead. | |
| // Partial clone config left behind by --filter=blob:none would cause checkout | |
| // to lazily fetch objects by SHA, which GitHub also rejects. | |
| fmt.Printf(" %-20s direct SHA fetch failed, falling back to full clone\n", t.ID) | |
| _ = os.RemoveAll(repoDir) | |
| fallbackCtx, fallbackCancel := context.WithTimeout(ctx, 10*time.Minute) | |
| defer fallbackCancel() | |
| cloneArgs := []string{"clone", "--quiet", t.Repo, repoDir} | |
| if out, err := exec.CommandContext(fallbackCtx, "git", cloneArgs...).CombinedOutput(); err != nil { | |
| return nil, fmt.Errorf("git clone fallback: %w\n%s", err, out) | |
| } | |
| } | |
| if out, err := exec.CommandContext(cloneCtx, "git", "-C", repoDir, "checkout", "--quiet", t.BaseCommit).CombinedOutput(); err != nil { | |
| return nil, fmt.Errorf("git checkout: %w\n%s", err, out) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bench-swe/internal/runner/runner.go` around lines 75 - 89, The fetch+fallback
share cloneCtx so a slow initial exec.CommandContext call can leave the fallback
using an almost-expired context; change the fallback path to (1) detect if the
first fetch error was a context cancellation/deadline (errors.Is(err,
context.Canceled) || errors.Is(err, context.DeadlineExceeded)) and if so return
that original fetch error instead of trying a retry, and (2) when proceeding
with the clone fallback, create a fresh context with an appropriate timeout
(e.g. via context.WithTimeout) and use that new context for exec.CommandContext
for the "git clone" and subsequent "git -C ... checkout" commands (refer to
shallowArgs, cloneCtx, and the cloneArgs/checkout exec.CommandContext calls).
Comment on lines
+12
to
+13
| "setup_commands": ["npm install"], | ||
| "test_command": "npx vitest run", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Task definition:"
jq -r '.setup_commands, .test_command' bench-swe/tasks/svelte/hard.json
echo
echo "Evidence from benchmark logs:"
rg -n "Tests no tests|Executable doesn't exist|Cannot find base config file|npx playwright install" \
bench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-baseline-tests.txt \
bench-results/swe-20260416-221127-ollama-jina-embeddings-v2-base-code/svelte-hard-with-lumen-tests.txtRepository: ory/lumen
Length of output: 2543
Benchmark task setup is incomplete; the test run exits with no tests discovered.
The current setup (lines 12–13) allows runs where Vitest reports Tests no tests due to two missing setup steps:
- SvelteKit configuration not generated: The warning "Cannot find base config file
./.svelte-kit/tsconfig.json" indicatesnpm run prepareornpx svelte-kit synchas not run. - Playwright browsers missing: The error "Executable doesn't exist" and the explicit log suggestion "npx playwright install" confirm browsers are not installed.
Both benchmark runs fail identically, making results unreliable for this task.
🔧 Proposed setup fix
- "setup_commands": ["npm install"],
+ "setup_commands": [
+ "npm install",
+ "npm run prepare || npx svelte-kit sync",
+ "npx playwright install chromium"
+ ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bench-swe/tasks/svelte/hard.json` around lines 12 - 13, The benchmark's
setup_commands are incomplete: update the setup sequence referenced by the
"setup_commands" entry so it installs dependencies, generates SvelteKit config,
and installs Playwright browsers before running tests (e.g., run npm install,
then npm run prepare or npx svelte-kit sync to create .svelte-kit/tsconfig.json,
and npx playwright install to fetch browsers); leave the "test_command" as "npx
vitest run". Ensure these steps are run in that order so SvelteKit base config
and Playwright browser executables exist before Vitest executes.
Comment on lines
+77
to
+79
| if chunks[0].StartLine < 2 { | ||
| t.Errorf("expected file-relative line numbers (>= 2), got StartLine=%d for first chunk", chunks[0].StartLine) | ||
| } |
There was a problem hiding this comment.
Prevent test panic on empty chunk output.
Line 77 dereferences chunks[0] without a length check; if symbol extraction regresses, this test panics instead of reporting a clear assertion failure.
Proposed fix
- if chunks[0].StartLine < 2 {
- t.Errorf("expected file-relative line numbers (>= 2), got StartLine=%d for first chunk", chunks[0].StartLine)
- }
+ if len(chunks) == 0 {
+ t.Fatal("expected at least one chunk from script symbols")
+ }
+ if chunks[0].StartLine < 2 {
+ t.Errorf("expected file-relative line numbers (>= 2), got StartLine=%d for first chunk", chunks[0].StartLine)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if chunks[0].StartLine < 2 { | |
| t.Errorf("expected file-relative line numbers (>= 2), got StartLine=%d for first chunk", chunks[0].StartLine) | |
| } | |
| if len(chunks) == 0 { | |
| t.Fatal("expected at least one chunk from script symbols") | |
| } | |
| if chunks[0].StartLine < 2 { | |
| t.Errorf("expected file-relative line numbers (>= 2), got StartLine=%d for first chunk", chunks[0].StartLine) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/chunker/svelte_test.go` around lines 77 - 79, The test dereferences
chunks[0] without checking length; modify the test in
internal/chunker/svelte_test.go to first assert len(chunks) > 0 (use t.Fatalf or
t.Errorf with a clear message like "expected at least one chunk, got 0") and
return on failure, then perform the existing StartLine assertion on chunks[0];
this prevents a panic if symbol extraction yields no chunks while preserving the
original check on chunks[0].StartLine.
Comment on lines
+63
to
+84
| {#if duration !== Infinity} | ||
| <div class="flex items-center gap-2"> | ||
| <span class="text-xs">{format(time)}</span> | ||
| <div | ||
| class="relative h-2 flex-1 rounded-full bg-gray-200 dark:bg-gray-700" | ||
| role="slider" | ||
| aria-label="Seek" | ||
| aria-valuenow={time} | ||
| aria-valuemin={0} | ||
| aria-valuemax={duration} | ||
| tabindex="0" | ||
| onpointerdown={() => { | ||
| paused = true; | ||
| }} | ||
| onpointerup={seek} | ||
| > | ||
| <div | ||
| class="absolute inset-0 h-full bg-gray-400 dark:bg-gray-600" | ||
| style="width: {(time / duration) * 100}%" | ||
| ></div> | ||
| </div> | ||
| <span class="text-xs">{duration ? format(duration) : "--:--"}</span> |
There was a problem hiding this comment.
Guard progress math when duration is zero or non-finite.
The current condition still renders the progress bar when duration is 0, which can yield invalid width values (NaN% / Infinity%) and a broken initial UI state.
💡 Proposed fix
- {`#if` duration !== Infinity}
+ {`#if` Number.isFinite(duration) && duration > 0}
<div class="flex items-center gap-2">
<span class="text-xs">{format(time)}</span>
<div
@@
<div
class="absolute inset-0 h-full bg-gray-400 dark:bg-gray-600"
- style="width: {(time / duration) * 100}%"
+ style="width: {Math.min(100, Math.max(0, (time / duration) * 100))}%"
></div>
</div>
<span class="text-xs">{duration ? format(duration) : "--:--"}</span>
</div>
{/if}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {#if duration !== Infinity} | |
| <div class="flex items-center gap-2"> | |
| <span class="text-xs">{format(time)}</span> | |
| <div | |
| class="relative h-2 flex-1 rounded-full bg-gray-200 dark:bg-gray-700" | |
| role="slider" | |
| aria-label="Seek" | |
| aria-valuenow={time} | |
| aria-valuemin={0} | |
| aria-valuemax={duration} | |
| tabindex="0" | |
| onpointerdown={() => { | |
| paused = true; | |
| }} | |
| onpointerup={seek} | |
| > | |
| <div | |
| class="absolute inset-0 h-full bg-gray-400 dark:bg-gray-600" | |
| style="width: {(time / duration) * 100}%" | |
| ></div> | |
| </div> | |
| <span class="text-xs">{duration ? format(duration) : "--:--"}</span> | |
| {`#if` Number.isFinite(duration) && duration > 0} | |
| <div class="flex items-center gap-2"> | |
| <span class="text-xs">{format(time)}</span> | |
| <div | |
| class="relative h-2 flex-1 rounded-full bg-gray-200 dark:bg-gray-700" | |
| role="slider" | |
| aria-label="Seek" | |
| aria-valuenow={time} | |
| aria-valuemin={0} | |
| aria-valuemax={duration} | |
| tabindex="0" | |
| onpointerdown={() => { | |
| paused = true; | |
| }} | |
| onpointerup={seek} | |
| > | |
| <div | |
| class="absolute inset-0 h-full bg-gray-400 dark:bg-gray-600" | |
| style="width: {Math.min(100, Math.max(0, (time / duration) * 100))}%" | |
| ></div> | |
| </div> | |
| <span class="text-xs">{duration ? format(duration) : "--:--"}</span> | |
| </div> | |
| {/if} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@testdata/fixtures/svelte/audio-player.svelte` around lines 63 - 84, The
progress bar math can produce NaN/Infinity when duration is 0 or non-finite;
update the conditional and width calculation around the slider so it only
renders / computes when duration is a positive finite number (e.g., replace the
{`#if` duration !== Infinity} check with a guard like duration > 0 &&
isFinite(duration)) and ensure the inline style for width (currently "width:
{(time / duration) * 100}%") uses a safe fallback (0%) when duration is
zero/non-finite; also adjust related aria-valuemax/aria-valuenow uses of
duration/time in the same block to avoid invalid accessibility values (affecting
the div role="slider", the style width, and aria-valuemax variables such as
duration, and the seek/onpointer handlers).
Comment on lines
+66
to
+78
| <div | ||
| class="relative h-2 flex-1 rounded-full bg-gray-200 dark:bg-gray-700" | ||
| role="slider" | ||
| aria-label="Seek" | ||
| aria-valuenow={time} | ||
| aria-valuemin={0} | ||
| aria-valuemax={duration} | ||
| tabindex="0" | ||
| onpointerdown={() => { | ||
| paused = true; | ||
| }} | ||
| onpointerup={seek} | ||
| > |
There was a problem hiding this comment.
Custom slider is not keyboard-operable.
With role="slider" and tabindex="0", keyboard controls are expected. Right now only pointer input works, which blocks keyboard-only users from seeking.
♿ Proposed fix
+ function handleSeekKeydown(e: KeyboardEvent) {
+ if (!Number.isFinite(duration) || duration <= 0) return;
+ const step = Math.max(1, duration / 20);
+ if (e.key === "ArrowLeft") {
+ e.preventDefault();
+ time = Math.max(0, time - step);
+ } else if (e.key === "ArrowRight") {
+ e.preventDefault();
+ time = Math.min(duration, time + step);
+ } else if (e.key === "Home") {
+ e.preventDefault();
+ time = 0;
+ } else if (e.key === "End") {
+ e.preventDefault();
+ time = duration;
+ }
+ }
@@
<div
class="relative h-2 flex-1 rounded-full bg-gray-200 dark:bg-gray-700"
role="slider"
@@
tabindex="0"
+ onkeydown={handleSeekKeydown}
onpointerdown={() => {
paused = true;
}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div | |
| class="relative h-2 flex-1 rounded-full bg-gray-200 dark:bg-gray-700" | |
| role="slider" | |
| aria-label="Seek" | |
| aria-valuenow={time} | |
| aria-valuemin={0} | |
| aria-valuemax={duration} | |
| tabindex="0" | |
| onpointerdown={() => { | |
| paused = true; | |
| }} | |
| onpointerup={seek} | |
| > | |
| function handleSeekKeydown(e: KeyboardEvent) { | |
| if (!Number.isFinite(duration) || duration <= 0) return; | |
| const step = Math.max(1, duration / 20); | |
| if (e.key === "ArrowLeft") { | |
| e.preventDefault(); | |
| time = Math.max(0, time - step); | |
| } else if (e.key === "ArrowRight") { | |
| e.preventDefault(); | |
| time = Math.min(duration, time + step); | |
| } else if (e.key === "Home") { | |
| e.preventDefault(); | |
| time = 0; | |
| } else if (e.key === "End") { | |
| e.preventDefault(); | |
| time = duration; | |
| } | |
| } | |
| <div | |
| class="relative h-2 flex-1 rounded-full bg-gray-200 dark:bg-gray-700" | |
| role="slider" | |
| aria-label="Seek" | |
| aria-valuenow={time} | |
| aria-valuemin={0} | |
| aria-valuemax={duration} | |
| tabindex="0" | |
| onkeydown={handleSeekKeydown} | |
| onpointerdown={() => { | |
| paused = true; | |
| }} | |
| onpointerup={seek} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@testdata/fixtures/svelte/audio-player.svelte` around lines 66 - 78, The
custom slider div with role="slider" (using aria-valuenow={time},
aria-valuemin/min, tabindex="0") is not keyboard-operable; add keyboard handling
to the element (e.g., an onkeydown handler on the same div) in the audio-player
component to respond to ArrowLeft/ArrowRight (small step), PageUp/PageDown
(larger step), Home/End (min/max) keys, update the time value and aria-valuenow
accordingly, call the existing seek(time) function to apply the change,
preventDefault for handled keys, and ensure interaction with the paused variable
and existing onpointerdown/onpointerup behavior remains consistent.
Comment on lines
+43
to
+57
| (async () => { | ||
| updateDebouncer.startRender(); | ||
| const processed = await processBlocks(content, sources); | ||
| handleBlocks(processed, requestId); | ||
| })(); | ||
| }); | ||
|
|
||
| onMount(() => { | ||
| if (typeof Worker !== "undefined") { | ||
| worker = new MarkdownWorker(); | ||
| worker.onmessage = (event: MessageEvent) => { | ||
| const data = event.data as { type?: string; blocks?: BlockToken[]; requestId?: number }; | ||
| if (data?.type !== "processed" || !data.blocks || data.requestId === undefined) return; | ||
| handleBlocks(data.blocks, data.requestId); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "markdown-renderer.svelte" -type fRepository: ory/lumen
Length of output: 104
🏁 Script executed:
cat -n ./testdata/fixtures/svelte/markdown-renderer.svelteRepository: ory/lumen
Length of output: 2583
🏁 Script executed:
find . -name "*debouncer*" -o -name "*updates*" | grep -E "\.(ts|js|svelte)$" | head -20Repository: ory/lumen
Length of output: 35
🏁 Script executed:
rg "updateDebouncer" --type ts --type js -A 3 -B 3Repository: ory/lumen
Length of output: 35
🏁 Script executed:
rg "updateDebouncer" -A 3 -B 3 | head -100Repository: ory/lumen
Length of output: 2119
🏁 Script executed:
find . -path "*/lib/utils/updates*" -type fRepository: ory/lumen
Length of output: 35
🏁 Script executed:
rg "export.*updateDebouncer|export.*debouncer" -A 5 | head -50Repository: ory/lumen
Length of output: 35
🏁 Script executed:
find . -name "*.ts" -o -name "*.js" | xargs rg "class.*Debouncer|debouncer\s*=" | head -20Repository: ory/lumen
Length of output: 35
🏁 Script executed:
git ls-files | grep -i update | head -20Repository: ory/lumen
Length of output: 35
🏁 Script executed:
ls -la testdata/fixtures/svelte/Repository: ory/lumen
Length of output: 819
Add error handling to prevent stuck render state.
The async code path (line 45) lacks error handling for processBlocks(), and the worker path has no error handlers. If either fails, updateDebouncer.endRender() will never execute, leaving the render state stuck.
Suggested fix
$effect(() => {
if (!browser) return;
const requestId = ++latestRequestId;
if (worker) {
updateDebouncer.startRender();
worker.postMessage({ type: "process", content, sources, requestId });
return;
}
(async () => {
updateDebouncer.startRender();
- const processed = await processBlocks(content, sources);
- handleBlocks(processed, requestId);
+ try {
+ const processed = await processBlocks(content, sources);
+ handleBlocks(processed, requestId);
+ } catch {
+ if (requestId === latestRequestId) updateDebouncer.endRender();
+ }
})();
});
onMount(() => {
if (typeof Worker !== "undefined") {
worker = new MarkdownWorker();
+ worker.onerror = () => {
+ updateDebouncer.endRender();
+ };
+ worker.onmessageerror = () => {
+ updateDebouncer.endRender();
+ };
worker.onmessage = (event: MessageEvent) => {
const data = event.data as { type?: string; blocks?: BlockToken[]; requestId?: number };
if (data?.type !== "processed" || !data.blocks || data.requestId === undefined) return;
handleBlocks(data.blocks, data.requestId);
};
}
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| (async () => { | |
| updateDebouncer.startRender(); | |
| const processed = await processBlocks(content, sources); | |
| handleBlocks(processed, requestId); | |
| })(); | |
| }); | |
| onMount(() => { | |
| if (typeof Worker !== "undefined") { | |
| worker = new MarkdownWorker(); | |
| worker.onmessage = (event: MessageEvent) => { | |
| const data = event.data as { type?: string; blocks?: BlockToken[]; requestId?: number }; | |
| if (data?.type !== "processed" || !data.blocks || data.requestId === undefined) return; | |
| handleBlocks(data.blocks, data.requestId); | |
| }; | |
| (async () => { | |
| updateDebouncer.startRender(); | |
| try { | |
| const processed = await processBlocks(content, sources); | |
| handleBlocks(processed, requestId); | |
| } catch { | |
| if (requestId === latestRequestId) updateDebouncer.endRender(); | |
| } | |
| })(); | |
| }); | |
| onMount(() => { | |
| if (typeof Worker !== "undefined") { | |
| worker = new MarkdownWorker(); | |
| worker.onerror = () => { | |
| updateDebouncer.endRender(); | |
| }; | |
| worker.onmessageerror = () => { | |
| updateDebouncer.endRender(); | |
| }; | |
| worker.onmessage = (event: MessageEvent) => { | |
| const data = event.data as { type?: string; blocks?: BlockToken[]; requestId?: number }; | |
| if (data?.type !== "processed" || !data.blocks || data.requestId === undefined) return; | |
| handleBlocks(data.blocks, data.requestId); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@testdata/fixtures/svelte/markdown-renderer.svelte` around lines 43 - 57, The
render can get stuck because the async IIFE that calls processBlocks and the
worker message path lack error handling and never call
updateDebouncer.endRender() on failure; wrap the async IIFE body that calls
processBlocks(content, sources) and handleBlocks(...) in try/catch/finally and
call updateDebouncer.endRender() in the finally block (ensure errors are logged
or forwarded), and add robust error handling for the worker path by adding
worker.onerror and/or wrapping worker.onmessage handler logic (the code that
inspects data and calls handleBlocks) in try/catch so that on exceptions you
call updateDebouncer.endRender() and log the error; reference processBlocks,
handleBlocks, updateDebouncer.startRender()/endRender(), MarkdownWorker, and
worker.onmessage when making these changes.
Comment on lines
+49
to
+101
| async function startRecording() { | ||
| try { | ||
| const stream = await navigator.mediaDevices.getUserMedia({ | ||
| audio: { | ||
| channelCount: 1, | ||
| sampleRate: 16000, // Whisper prefers 16kHz | ||
| echoCancellation: true, | ||
| noiseSuppression: true, | ||
| }, | ||
| }); | ||
|
|
||
| mediaStream = stream; | ||
|
|
||
| // Set up audio context for visualization | ||
| audioContext = new AudioContext(); | ||
| const source = audioContext.createMediaStreamSource(stream); | ||
| analyser = audioContext.createAnalyser(); | ||
| analyser.fftSize = 64; // Small for performance, gives 32 frequency bins | ||
| analyser.smoothingTimeConstant = 0.4; | ||
| source.connect(analyser); | ||
| frequencyData = new Uint8Array(analyser.frequencyBinCount); | ||
|
|
||
| // Start MediaRecorder | ||
| // Use webm/opus for broad browser support | ||
| const mimeType = MediaRecorder.isTypeSupported("audio/webm;codecs=opus") | ||
| ? "audio/webm;codecs=opus" | ||
| : "audio/webm"; | ||
|
|
||
| mediaRecorder = new MediaRecorder(stream, { mimeType }); | ||
| audioChunks = []; | ||
|
|
||
| mediaRecorder.ondataavailable = (e) => { | ||
| if (e.data.size > 0) { | ||
| audioChunks = [...audioChunks, e.data]; | ||
| } | ||
| }; | ||
|
|
||
| mediaRecorder.start(100); // Collect data every 100ms | ||
| startVisualization(); | ||
| } catch (err) { | ||
| if (err instanceof DOMException) { | ||
| if (err.name === "NotAllowedError") { | ||
| onerror("Microphone access denied. Please allow in browser settings."); | ||
| } else if (err.name === "NotFoundError") { | ||
| onerror("No microphone found."); | ||
| } else { | ||
| onerror(`Microphone error: ${err.message}`); | ||
| } | ||
| } else { | ||
| onerror("Could not access microphone."); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle late startRecording() resolution after unmount.
If the component unmounts before getUserMedia resolves, the current code can still initialize recorder/audio resources after destroy.
🧯 Proposed fix
+ let destroyed = false;
+
async function startRecording() {
try {
const stream = await navigator.mediaDevices.getUserMedia({
@@
});
+ if (destroyed) {
+ stream.getTracks().forEach((track) => track.stop());
+ return;
+ }
+
mediaStream = stream;
@@
onDestroy(() => {
+ destroyed = true;
// Fire and forget - cleanup happens but we don't wait
stopRecording();
});Also applies to: 160-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@testdata/fixtures/svelte/voice-recorder.svelte` around lines 49 - 101, The
async startRecording function can finish after the component is unmounted and
then initialize audio resources; add a mounted/aborted guard immediately before
and after awaiting navigator.mediaDevices.getUserMedia to ensure the component
is still live — if the guard shows unmounted, stop and release the obtained
stream (stop its tracks), do not assign mediaStream, do not create audioContext
or mediaRecorder, and return early; apply the same mount-check/early-abort
pattern to the other async block mentioned (lines 160-167) so functions that
await getUserMedia or other async setup verify the component is still mounted
before creating or assigning media resources (mediaStream, audioContext,
mediaRecorder, startVisualization).
Comment on lines
+103
to
+139
| function stopRecording(): Promise<Blob | null> { | ||
| return new Promise((resolve) => { | ||
| stopVisualization(); | ||
|
|
||
| // Stop all audio tracks | ||
| if (mediaStream) { | ||
| mediaStream.getTracks().forEach((track) => track.stop()); | ||
| mediaStream = null; | ||
| } | ||
|
|
||
| // Close audio context | ||
| if (audioContext) { | ||
| audioContext.close(); | ||
| audioContext = null; | ||
| } | ||
| analyser = null; | ||
|
|
||
| if (!mediaRecorder || mediaRecorder.state === "inactive") { | ||
| mediaRecorder = null; | ||
| resolve( | ||
| audioChunks.length > 0 | ||
| ? new Blob(audioChunks, { type: audioChunks[0]?.type || "audio/webm" }) | ||
| : null | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| // Wait for final data before resolving | ||
| mediaRecorder.onstop = () => { | ||
| const mimeType = audioChunks[0]?.type || "audio/webm"; | ||
| const blob = audioChunks.length > 0 ? new Blob(audioChunks, { type: mimeType }) : null; | ||
| mediaRecorder = null; | ||
| resolve(blob); | ||
| }; | ||
|
|
||
| mediaRecorder.stop(); | ||
| }); |
There was a problem hiding this comment.
Deduplicate stopRecording() to prevent stop-state races.
stopRecording() can be triggered from multiple paths; repeated calls can overwrite onstop and race mediaRecorder.stop(), causing unreliable resolution behavior.
🛠️ Proposed fix
+ let stopPromise: Promise<Blob | null> | null = null;
+
function stopRecording(): Promise<Blob | null> {
- return new Promise((resolve) => {
+ if (stopPromise) return stopPromise;
+ stopPromise = new Promise((resolve) => {
+ const finalize = (blob: Blob | null) => {
+ stopPromise = null;
+ resolve(blob);
+ };
stopVisualization();
@@
if (!mediaRecorder || mediaRecorder.state === "inactive") {
mediaRecorder = null;
- resolve(
+ finalize(
audioChunks.length > 0
? new Blob(audioChunks, { type: audioChunks[0]?.type || "audio/webm" })
: null
);
return;
}
@@
mediaRecorder.onstop = () => {
const mimeType = audioChunks[0]?.type || "audio/webm";
const blob = audioChunks.length > 0 ? new Blob(audioChunks, { type: mimeType }) : null;
mediaRecorder = null;
- resolve(blob);
+ finalize(blob);
};
mediaRecorder.stop();
});
+ return stopPromise;
}Also applies to: 142-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@testdata/fixtures/svelte/voice-recorder.svelte` around lines 103 - 139,
stopRecording() can be called multiple times causing races by overwriting
mediaRecorder.onstop and calling mediaRecorder.stop() repeatedly; fix by making
stopRecording idempotent: add an internal single shared stopping promise/flag
(e.g., stopPromise or isStopping) inside the module so subsequent calls return
the same Promise and early-return if stopping is in progress, ensure you only
set mediaRecorder.onstop once and call mediaRecorder.stop() only when not
already stopping, and keep existing cleanup steps (stopVisualization,
mediaStream tracks stop, audioContext.close, analyser nulling, mediaRecorder
nulling, resolving with Blob from audioChunks) but funnel all callers through
that single in-flight stop promise for stopRecording, and apply the same pattern
to the other stop routine at lines 142-167.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #126
Summary
.sveltetosupportedExtensionsandDefaultLanguagesso Svelte files are no longer silently skipped by the merkle walker. On large SvelteKit monorepos this can recover 40%+ of previously invisible source files.SvelteChunker(newinternal/chunker/svelte.go) using a two-phase injection pattern: the outer tree-sitter Svelte grammar locates<script>elements; each block'sraw_textis re-parsed with the existing TypeScriptTreeSitterChunkerto extract named symbols. Line numbers are adjusted to be file-relative so search results point to the correct lines.$state(),$derived()) parse cleanly as TypeScript call-expression initializers — no special handling needed.text-embedding-voyage-4-nanoinKnownModels(LM Studio backend, 1024 dims, 2048 ctx).cmdhook tests that failed when~/.config/lumen/config.yamlset a non-default model: addsXDG_CONFIG_HOMEisolation so bothwriteHookTestDBand the hook under test resolve the same default model, preventing DB-path hash mismatches and dimension-mismatch schema resets that silently wipedlast_indexed_at.Files changed
internal/chunker/svelte.goSvelteChunkerwith two-phase parseinternal/chunker/svelte_test.gointernal/chunker/languages.go.svelteinsupportedExtensions+DefaultLanguagesinternal/chunker/treesitter_test.go.sveltefixture totrivialSourcesinternal/models/models.gotext-embedding-voyage-4-nanotoKnownModelsinternal/models/models_test.gocmd/hook_test.goXDG_CONFIG_HOMEin three tests to isolate from user configgo.mod/go.sumgithub.com/alexaandru/go-sitter-forest/svelte v1.9.2Test plan
go test ./...— all 12 packages passTestSvelteChunker_ScriptSymbols— verifies function/interface/class extraction and file-relative line numbersTestSvelteChunker_EmptyScript/TestSvelteChunker_NoScript— edge cases return zero chunks without errorTestDefaultLanguages_AllExtensionsPresent—.sveltefixture added totrivialSourcesXDG_CONFIG_HOMEisolation🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests