Full virtual scrolling for entry list #2129
Conversation
github-actions
bot
added
💻 FW Lite
📝 WalkthroughWalkthroughAdds entry indexing API ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@frontend/viewer/eslint.config.js`:
- Around line 65-69: The '@typescript-eslint/naming-convention' rule entry has
an invalid modifier token: the 'modifiers' array uses a single string 'static
readonly' which breaks ESLint schema validation; update the 'modifiers' property
in the rule object (the one with 'selector': 'default') to use separate modifier
strings (e.g., ['const'] or ['static','readonly'] as appropriate) and if you
need different semantics add a separate rule object for the
['static','readonly'] case so each modifier is its own array element.
In `@frontend/viewer/src/lib/services/entry-loader-service.svelte.ts`:
- Around line 330-348: In the private async method `#runQuietResetOnce`, remove
the stray debug statement console.log(entries[0]) so no debug output is left in
production; simply delete that line in the function (the block that awaits
Promise.all, checks generation, and calls `#swapCachesForQuietReset` should remain
unchanged).
- Around line 308-318: The RangeError message in the private method
`#markBatchRequested` currently says "Batch number must be positive" but the check
allows zero; update the error text to reflect that zero is valid (e.g., "Batch
number must be non-negative" or "Batch number must not be negative") so the
message matches the condition (keep the same check logic in `#markBatchRequested`
and only change the thrown message).
- Around line 350-353: In `#fetchEntriesForQuietReset`, the call batches.sort()
performs lexicographic sorting causing incorrect offset for multi-digit batch
indices; change the code to determine the numeric minimum batch (e.g., use a
numeric comparator like (a,b)=>a-b on a copy of batches or use
Math.min(...batches)) and compute offset = minBatch * this.batchSize, then call
this.#fetchRange(offset, this.batchSize * batches.length) so offsets are correct
for batches ≥ 10.
- Around line 355-379: In `#swapCachesForQuietReset` the code uses
batches.sort()[0], which sorts lexicographically and mutates the array — replace
that with a numeric, non-mutating selection (e.g., use Math.min(...batches) or
batches.slice().sort((a,b)=>a-b)[0]) to compute offset correctly; also remove
the debug console.log('QUIET RESET COMPLETE') at the end of the method. Ensure
you update the offset calculation and delete the debug log inside the
`#swapCachesForQuietReset` method (consistent with similar fix in
`#fetchEntriesForQuietReset`).
- Around line 146-156: Save the current generation into a local variable at the
start of getOrLoadEntryIndex (e.g., const currentGen = this.#generation) before
calling await this.#api.getEntryIndex(...); after the await, compare currentGen
to this.#generation and if they differ do not cache the returned index or
overwrite this.#idToIndex (simply return the existing cached value from
this.#idToIndex.get(id) or undefined), otherwise continue to set
this.#idToIndex.set(id, index) and return the index; reference
getOrLoadEntryIndex, this.#idToIndex, this.#generation, this.#buildQueryOptions,
this.#deps.search, and this.#api.getEntryIndex when applying the change.
In `@frontend/viewer/src/project/browse/EntriesList.svelte`:
- Around line 104-112: The current $effect launches an async block that awaits
entryLoader?.getOrLoadEntryIndex(selectedEntryId) and then calls untrack(() =>
void scrollToEntry(vList!, selectedEntryId)), which can scroll to a stale
selectedEntryId if it changes while awaiting; fix by capturing the current
selectedEntryId into a local const (e.g., const targetId = selectedEntryId)
before the await, and before calling scrollToEntry verify that selectedEntryId
=== targetId (or that the entry still exists) to avoid acting on a stale id;
update the async block around entryLoader.getOrLoadEntryIndex, scrollToEntry,
and vList references (symbols: $effect, selectedEntryId,
entryLoader.getOrLoadEntryIndex, scrollToEntry, vList, untrack) accordingly.
In `@frontend/viewer/src/project/tasks/TaskView.svelte`:
- Around line 68-71: Wrap the async call to entriesList?.selectNextEntry()
inside a try/catch inside the onNextEntry handler to prevent unhandled
rejections: call selectNextEntry() in try, check the returned next as before,
and in catch log the error and invoke a clear fallback such as calling onDone()
or triggering your app's toast/error handler (use the existing toast/error
utility if available) so the UI doesn't remain stuck; update the onNextEntry
anonymous handler accordingly (referencing entriesList.selectNextEntry and
onDone).
In `@frontend/viewer/tests/entries-list-v2.test.ts`:
- Around line 288-292: The test currently uses an unsafe double-cast ('Headword'
as unknown as SortField) inside page.evaluate; instead, pass the SortField enum
value from the Node test scope into the browser context and use it in the call
to testUtils.demoApi.getEntries (e.g., call page.evaluate with SortField as an
argument), so replace the inline cast with a parameterized value derived from
the imported SortField and reference it inside the page.evaluate callback when
invoking getEntries.
In `@frontend/viewer/tests/entries-list.test.ts`:
- Around line 114-139: The test "scroll to specific entry maintains entry
visibility" currently may assert toContainText('') when selectedText is
null/short; ensure you capture a non-empty selectedText from firstEntry (use
getLocators/firstEntry), assert selectedText is truthy (fail the test early if
not), normalize it (trim) and compute an expected snippet as either the full
selectedText when length < 10 or selectedText.slice(0,10) otherwise, then use
that expected snippet in the expect on firstEntryAfter
(entryRows/firstEntryAfter) so the assertion never trivially passes with an
empty string.
🧹 Nitpick comments (15)
frontend/viewer/src/project/browse/sort/options.ts (1)
1-9: LGTM - Clean extraction of sort configuration.Good refactoring to centralize sort options in a dedicated module. The
as constassertion enables proper type inference forSortConfig, and usingSortFieldfrom generated types maintains type safety with the backend.Consider using a stricter type for
dirto prevent typos and improve autocomplete:type SortDirection = 'asc' | 'desc'; export const sortOptions = [ {field: SortField.SearchRelevance, dir: 'asc' as SortDirection}, {field: SortField.Headword, dir: 'asc' as SortDirection}, {field: SortField.Headword, dir: 'desc' as SortDirection} ] as const;Alternatively, if
SortDirectionexists in the generated types, consider importing it from there.backend/FwLite/LcmCrdt/Data/MiniLcmRepository.cs (1)
263-281: Performance concern: full entity loading during linear scan.The fallback implementation is acknowledged as sub-optimal, but the current approach loads full
Entryobjects with all related data (Senses, ExampleSentences, PartOfSpeech, ComplexForms, Components) just to find an index. For large datasets, this is both CPU and memory intensive.Consider a lighter-weight query that only selects entry IDs in sorted order, then finds the index:
♻️ Suggested optimization
public async Task<int> GetEntryIndex(Guid entryId, string? query = null, QueryOptions? options = null) { - // This is a fallback implementation that's not optimal for large datasets, - // but it works correctly. Ideally, we'd use ROW_NUMBER() window function with linq2db - // for better performance on large entry lists. For now, we enumerate through sorted entries - // and count until we find the target entry. - - var rowIndex = 0; - await foreach (var entry in GetEntries(query, options)) - { - if (entry.Id == entryId) - { - return rowIndex; - } - rowIndex++; - } - - return -1; + // Fallback: enumerate IDs only (avoids loading full entities) + options = await EnsureWritingSystemIsPopulated(options ?? QueryOptions.Default); + var queryable = Entries; + (queryable, var sortingHandled) = await FilterEntries(queryable, query, options, options.Order); + if (!sortingHandled) + queryable = await ApplySorting(queryable, options, query); + + var rowIndex = 0; + await foreach (var id in queryable.Select(e => e.Id).AsAsyncEnumerable()) + { + if (id == entryId) return rowIndex; + rowIndex++; + } + return -1; }frontend/viewer/src/lib/components/Delayed.svelte (1)
76-80: Minor: redundant reset call.
startLoading()callsreset(), but the watch effect already callsreset()beforestartLoading(). This redundancy is harmless but could be removed for clarity.frontend/viewer/src/stories/primitives/delayed.stories.svelte (1)
31-59: Minor: timing measurement captures render time, not load start.The
{@conststart = Date.now()}is evaluated on each render of the snippet, soend - startwill be close to 0ms rather than the actual load duration. For demo purposes this is fine, but if accurate timing is desired, the start time would need to be captured before the load begins.frontend/viewer/src/project/demo/in-memory-demo-api.ts (1)
263-275: Silent failure when entry not found in createSense/createExampleSentence.If the entry doesn't exist,
entry?.senses.push(sense)silently does nothing, but the method still returns the sense as if it was created. This inconsistency could mask bugs during testing. Consider throwing an error likegetEntrydoes:♻️ Suggested fix
createSense(entryGuid: string, sense: ISense): Promise<ISense> { const entry = this._entries.find(e => e.id === entryGuid); - entry?.senses.push(sense); - if (entry) this.#projectEventBus.notifyEntryUpdated(entry); + if (!entry) throw new Error(`Entry ${entryGuid} not found`); + entry.senses.push(sense); + this.#projectEventBus.notifyEntryUpdated(entry); return Promise.resolve(sense); } createExampleSentence(entryGuid: string, senseGuid: string, exampleSentence: IExampleSentence): Promise<IExampleSentence> { const entry = this._entries.find(e => e.id === entryGuid); - entry?.senses.find(s => s.id === senseGuid)?.exampleSentences.push(exampleSentence); - if (entry) this.#projectEventBus.notifyEntryUpdated(entry); + if (!entry) throw new Error(`Entry ${entryGuid} not found`); + const sense = entry.senses.find(s => s.id === senseGuid); + if (!sense) throw new Error(`Sense ${senseGuid} not found`); + sense.exampleSentences.push(exampleSentence); + this.#projectEventBus.notifyEntryUpdated(entry); return Promise.resolve(exampleSentence); }backend/FwLite/MiniLcm.Tests/QueryEntryTestsBase.cs (1)
229-245: Make morph-type tests deterministic across implementations.These tests assume entries without an explicit MorphType are treated as Stem. If any backend returns null/unspecified instead, the OR filter could miss entries. Consider explicitly setting MorphType on test entries (or adding a Root entry) to avoid backend-specific defaults.
AGENTS.md (1)
51-88: Consider toning down repeated exclamation marks for consistency.The emphasis is useful, but the tone could read cleaner with fewer exclamation marks.
frontend/viewer/tests/entries-list.test.ts (3)
3-6: Clarify or remove the "vibe coded" comment.The comment "This file is vibe coded" is unclear and unprofessional for a test file that will be maintained long-term. Consider removing it or replacing it with a meaningful description of the test coverage.
46-66: ReplacewaitForTimeoutwith explicit condition waits to reduce flakiness.Using
page.waitForTimeout(500)is a Playwright anti-pattern that can cause flaky tests. Consider waiting for a specific condition instead.♻️ Suggested improvement
- await page.waitForTimeout(500); - - const scrollTop = await vlist.evaluate((el) => el.scrollTop); - expect(scrollTop).toBeGreaterThan(850); + // Wait for scroll to settle and content to render + await expect(async () => { + const scrollTop = await vlist.evaluate((el) => el.scrollTop); + expect(scrollTop).toBeGreaterThan(850); + }).toPass({timeout: 2000});
142-146: Consider extractingwaitForProjectViewReadyto a shared test utilities module.This helper function is duplicated in
entries-list-v2.test.tswith slight variations. Extracting it to a shared module would improve maintainability and ensure consistent behavior across test files.backend/FwLite/FwLiteWeb/Routes/MiniLcmRoutes.cs (1)
142-159: Consider adding input validation forstartandsizeparameters.Negative values for
startor non-positive values forsizecould cause unexpected behavior or exceptions downstream. Consider validating these parameters and returning appropriate error responses.♻️ Suggested validation
public static Task<EntryWindowResponse> GetEntriesWindow( [FromQuery] int start, [FromQuery] int size, [AsParameters] MiniLcmQueryOptions options, [FromServices] MiniLcmHolder holder) { + if (start < 0) return Task.FromResult(new EntryWindowResponse(start, [])); + if (size <= 0) return Task.FromResult(new EntryWindowResponse(start, [])); var api = holder.MiniLcmApi; return api.GetEntriesWindow(start, size, null, options.ToQueryOptions()); }frontend/viewer/tests/entries-list-v2.test.ts (2)
53-75: Proportional scroll calculation may be imprecise for variable-height items.The
scrollToIndexfunction calculates scroll position as a proportion of total scroll height. This works well for fixed-height items but may be imprecise if VList uses variable or estimated heights. The function signature includes_itemHeightbut doesn't use it.💡 Consider using the itemHeight parameter
- async function scrollToIndex(page: Page, targetIndex: number, _itemHeight: number): Promise<void> { + async function scrollToIndex(page: Page, targetIndex: number, itemHeight: number): Promise<void> { const {vlist} = getLocators(page); - // Get total count for proportional scrolling - const {totalCount} = await page.evaluate(async () => { - const api = window.__PLAYWRIGHT_UTILS__.demoApi; - return {totalCount: await api.countEntries()}; - }); - - // Use proportion of total scroll for accurate positioning - // This accounts for VList's estimated item heights across all items - const targetScroll = await vlist.evaluate((el, params) => { - const {idx, total} = params; - return (idx / total) * el.scrollHeight; - }, {idx: targetIndex, total: totalCount}); + // Use measured item height for more precise positioning + const targetScroll = targetIndex * itemHeight;Alternatively, if proportional scrolling is intentional due to VList's behavior, document this choice with a comment.
109-109: Empty lines can be removed for cleaner code.There are consecutive empty lines at lines 109-110 and 125-126 that could be consolidated.
Also applies to: 125-125
backend/FwLite/MiniLcm.Tests/EntryWindowTestsBase.cs (1)
155-169: Consider adding tests for edge cases with invalid inputs.The test suite covers normal operation well, but consider adding tests for:
- Negative
startvalue- Zero or negative
sizevalue- Very large
sizevalue (beyond MaxPageSize limit)FwDataMiniLcmApi validates these constraints and throws
ArgumentOutOfRangeException. Tests documenting this behavior would clarify expected API behavior for callers.frontend/viewer/src/lib/services/entry-loader-service.svelte.ts (1)
162-176: Minor inconsistency:removeEntryByIdis fire-and-forget whileupdateEntryawaits.Both methods trigger a quiet reset, but
removeEntryByIdusesvoid this.quietReset()(fire-and-forget) whileupdateEntryawaits it. If this is intentional, a brief comment explaining the difference would help maintainability.
myieye
force-pushed
the
feature/virtual-scrolling-entries
branch
from
fd070f2 to
949312c
Compare
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@AGENTS.md`:
- Around line 43-46: The bullet items in AGENTS.md use repeated exclamation
points in the critical-code notes; edit the bullets (`backend/FwLite/AGENTS.md`
and `backend/FwHeadless/AGENTS.md` entries shown in the list) to tone down
punctuation by replacing repeated exclamation marks with a single exclamation or
a period (e.g., "Critical code! Data loss risks." or "Critical code. Data loss
risks."), keeping the warning text but removing extra emphasis for better
readability.
In `@backend/LfClassicData/LfClassicMiniLcmApi.cs`:
- Around line 428-431: The GetEntryIndex method in LfClassicMiniLcmApi currently
throws NotImplementedException but the API route mapping in LfClassicRoutes.cs
doesn't expose /entry/{id:Guid}/index, so either implement the method or
explicitly document/declare it unsupported: if you choose to implement, add the
actual indexing logic inside LfClassicMiniLcmApi.GetEntryIndex and then add the
route in LfClassicRoutes.cs to map the endpoint; if you choose not to support
indexing for LfClassic, replace the NotImplementedException with a
NotSupportedException (or add an XML doc comment on
LfClassicMiniLcmApi.GetEntryIndex stating entry indexing is not supported for
LfClassic projects) so callers and maintainers know this is intentional.
In `@frontend/viewer/AGENTS.md`:
- Around line 18-21: The heading "### Generated .NET Types" contains a sentence
where "API interfaces" is redundant; update the sentence to simplify wording by
removing "API interfaces" and rephrasing to something like: "This project
depends on TypeScript types and APIs generated from .NET (via
`Reinforced.Typings`). If you change .NET models or `JSInvokable` APIs, rebuild
the backend to update these types." Locate the paragraph under the "###
Generated .NET Types" heading in AGENTS.md and replace the current sentence with
the simplified version while preserving the note about rebuilding after changing
.NET models or `JSInvokable` APIs.
In `@frontend/viewer/src/project/browse/EntriesList.svelte`:
- Around line 94-103: The module currently computes skeletonRowCount using
Math.random() at import time (skeletonRowCount) which causes SSR hydration
mismatches; change this to a deterministic initial value (e.g., const
skeletonRowCount = 10) and move any randomness into a client-only onMount block
that updates skeletonRowCount after mount (so the indexArray/$derived reacts to
the updated value), ensuring entryLoader, indexArray and the $derived logic
remain unchanged but rely on the reactive skeletonRowCount variable instead of a
randomized module-level value.
In `@frontend/viewer/src/project/browse/EntryView.svelte`:
- Around line 66-70: The restore() handler currently runs even when the user
lacks write access and doesn't refresh state after re-creating the entry; add a
write-guard at the top of restore() (e.g., check an existing permission flag
like canWrite or writeAccess and return if false) and after awaiting
miniLcmApi.createEntry(entry) trigger a refresh of the view (call the
component’s refetch function such as fetchEntry(entry.id) or fetchEntries(), or
emit a 'refresh' event via dispatch) before setting deleted = false so the UI
reflects the newly recreated entry; keep references to restore(), entry,
miniLcmApi.createEntry, deleted and the chosen refetch/dispatch function when
making the change.
- Around line 120-130: The pt call and the Restore translation use the wrong
syntax for Lingui extraction; update the pt invocation to use tagged templates
for the literal keys (replace pt('entry', 'word', $currentView) with
pt($t`entry`, $t`word`, $currentView`)) and change the Restore button
translation from function-call syntax ($t('Restore')) to the tagged-template
form ($t`Restore`), leaving the restore() onclick and surrounding Alert/Button
components unchanged.
In `@frontend/viewer/src/project/demo/in-memory-demo-api.ts`:
- Around line 105-109: The getEntryIndex implementation ignores requested sort
order by calling getFilteredSortedEntries with the wrong options type; update
getEntryIndex (and the similar implementation at the other location) to
propagate IIndexQueryOptions.order into the filtering/sorting step—either
convert/forward the IIndexQueryOptions to the IFilterQueryOptions expected by
getFilteredSortedEntries or overload/getFilteredSortedEntries to accept
IIndexQueryOptions so the provided order is applied before calling
entries.findIndex; ensure the same fix is applied to the duplicate method around
the other occurrence (the one currently mirroring getEntryIndex logic).
- Around line 88-91: The mockJsEventListener currently resolves immediately
causing tight polling loops; change nextEventAsync on mockJsEventListener to
mimic production long‑poll semantics by returning a never‑resolving promise
(e.g., new Promise(() => {})) instead of Promise.resolve(null!), keep lastEvent
as Promise.resolve(null) (or leave as-is) so consumers won't busy-loop; update
the IJsEventListener mock implementation for nextEventAsync accordingly.
In `@frontend/viewer/tests/entries-list-v2.test.ts`:
- Around line 258-262: The test updates only lexemeForm.seh which can leave the
UI headword (citationForm) unchanged and cause flakiness; inside the
page.evaluate block where you build updated and call
testUtils.demoApi.updateEntry(entry, updated), also set
updated.lexemeForm.citationForm (or updated.lexemeForm.en as appropriate) to the
same newLexemeForm so the citation/headword is mutated along with seh before
calling updateEntry; locate this in the page.evaluate closure where
newLexemeForm, updated, and testUtils.demoApi.updateEntry are used and add the
citationForm assignment to keep headword and seh in sync.
🧹 Nitpick comments (4)
frontend/viewer/src/lib/components/ui/alert/alert.svelte (1)
8-10: Confirm light‑mode border intent for destructive alerts.Line 9 drops
border-destructive/50, so light mode will inherit the default border color while using a destructive background. If you still want a destructive border in light mode, consider adding it back.💡 Optional tweak
- destructive: 'bg-destructive text-destructive-foreground dark:border-destructive-foreground/50 [&>svg]:text-destructive', + destructive: 'bg-destructive text-destructive-foreground border-destructive/50 dark:border-destructive-foreground/50 [&>svg]:text-destructive',backend/FwLite/AGENTS.md (1)
33-43: Good documentation addition for generated types.This section clearly explains the TypeScript type generation workflow and where to find configuration. Minor style nit: "API interfaces" on line 35 is slightly redundant—could simply say "APIs" or "types and interfaces."
backend/FwLite/MiniLcm.Tests/QueryEntryTestsBase.cs (1)
229-245: Consider setting MorphType explicitly in setup to avoid backend-default coupling.
These tests assume the default morph type is alwaysStem; if a backend changes defaults, this could become brittle. Making MorphType explicit in the test data would make the assertions more robust.frontend/viewer/tests/entry-edit-persists.test.ts (1)
17-25: Use shared readiness helper to avoid drift.There’s already a
waitForProjectViewReadyhelper infrontend/viewer/tests/test-utils.tswith optionalwaitForTestUtils. Reusing it keeps readiness checks consistent across Playwright suites.♻️ Suggested refactor
-import {type Page, expect, test} from '@playwright/test'; +import {type Page, expect, test} from '@playwright/test'; +import {waitForProjectViewReady} from './test-utils'; @@ - async function waitForProjectViewReady(page: Page) { - await expect(page.locator('.i-mdi-loading')).toHaveCount(0, {timeout: 10000}); - await page.waitForFunction(() => document.fonts.ready); - await expect(page.locator('[data-skeleton]')).toHaveCount(0, {timeout: 10000}); - } - - async function waitForTestUtils(page: Page) { - await page.waitForFunction(() => window.__PLAYWRIGHT_UTILS__?.demoApi, {timeout: 5000}); - } @@ test.beforeEach(async ({page}) => { await page.goto('/testing/project-view'); - await waitForProjectViewReady(page); - await waitForTestUtils(page); + await waitForProjectViewReady(page, true); });Also applies to: 44-48
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@frontend/viewer/AGENTS.md`:
- Around line 18-27: The README entry in AGENTS.md points to the wrong generated
types directory; update the path from "src/lib/generated-types/" to the correct
"src/lib/dotnet-types/generated-types/" and sanity-check the referenced index
file (frontend/viewer/src/lib/dotnet-types/index.ts) so the doc matches the
actual location of the generated TypeScript files.
In `@frontend/viewer/src/lib/services/ENTRY_LOADER_PLAN.md`:
- Around line 98-111: The fenced code block under "Batch Loading Logic" lacks a
language identifier which violates markdownlint MD040; update the
triple-backtick fence to include an appropriate language tag (for example change
``` to ```text) so the block becomes ```text ...```; ensure any other fenced
blocks in ENTRY_LOADER_PLAN.md also get suitable tags (e.g., typescript or
svelte where applicable) to satisfy the linter.
- Around line 168-197: Update the API surface in ENTRY_LOADER_PLAN.md so the
documented method names match the implementation: replace getEntryByIndex with
getCachedEntryByIndex, replace loadEntryByIndex with getOrLoadEntryByIndex, and
replace loadCount with loadInitialCount; ensure the signatures/comments remain
correct (sync vs async, batch-fetch behavior) and remove or mark the old names
to avoid confusion.
In `@frontend/viewer/src/lib/services/entry-loader-service.svelte.ts`:
- Around line 177-205: The current optimization wrongly ignores update events
for cached entries with cachedIndex < 0; instead of returning true (which skips
quietReset), treat a cached "not found" as potentially becoming relevant by not
short‑circuiting. In tryOptimizeUpdateEntryEvent, remove or change the early
return for the cachedIndex < 0 branch so it does not return true — let the
function fall through (return false) so updateEntry will call quietReset when
needed; reference symbols: tryOptimizeUpdateEntryEvent, cachedIndex,
updateEntry, quietReset.
In `@frontend/viewer/src/locales/fr.po`:
- Around line 626-630: Fill the empty msgstr entries for the listed msgid
strings with French translations: replace msgstr "" for "entry" with "entrée"
(or "article" if context prefers), for "word" use "mot", for "Filter" use
"Filtrer" (or "Filtre" depending on UI verb/noun context), for "Restore" use
"Restaurer", and for "This {0} was deleted" use "Ce {0} a été supprimé"; apply
these same translations to the other occurrences referenced (lines ~786-789,
1418-1422, 1702-1706, 1901-1905) so no msgid remains with an empty msgstr.
In `@frontend/viewer/src/stories/primitives/delayed.stories.svelte`:
- Around line 42-54: The timing is captured inside the {`#snippet`
children(state)} block so `@const start` is recomputed on every render and makes
`end - start` ~0; fix by capturing the start time when the load begins (e.g.,
set a `start` timestamp on the `state` object or a top-level variable when you
initiate the delay) and then read that persisted `state.start` inside the
snippet to compute `end - state.start`; update any references to `start`/`end`
accordingly (keep `end` local when displaying but use the persisted
`state.start` for the subtraction).
♻️ Duplicate comments (5)
AGENTS.md (1)
43-48: Excessive exclamation marks in critical-code bullets.The repeated exclamation marks in lines 45-46 could be toned down for readability, as flagged by static analysis.
backend/LfClassicData/LfClassicMiniLcmApi.cs (1)
428-431: Consider usingNotSupportedExceptioninstead ofNotImplementedException.Since entry indexing is intentionally not supported for LfClassic projects (and
GetEntryIndexis not exposed inLfClassicRoutes.cs),NotSupportedExceptionwould better convey intent.NotImplementedExceptiontypically suggests code that will be implemented later.♻️ Suggested change
public Task<int> GetEntryIndex(Guid entryId, string? query = null, IndexQueryOptions? options = null) { - throw new NotImplementedException(); + throw new NotSupportedException("Entry indexing is not supported for LfClassic projects."); }frontend/viewer/AGENTS.md (1)
20-20: Minor: "API interfaces" is redundant.Consider simplifying to "APIs" as flagged in previous review.
✏️ Suggested edit
-This project depends on TypeScript types and API interfaces generated from .NET (via `Reinforced.Typings`). If you change .NET models or `JSInvokable` APIs, you must rebuild the backend to update these types. +This project depends on TypeScript types and APIs generated from .NET (via `Reinforced.Typings`). If you change .NET models or `JSInvokable` APIs, you must rebuild the backend to update these types.frontend/viewer/src/project/demo/in-memory-demo-api.ts (2)
88-91: Mock JsEventListener should mimic long‑poll semantics.Resolving immediately can cause tight polling loops in consumers. Consider matching the production “never‑resolving” behavior.
Based on learnings, the default listener intentionally does not resolve to avoid busy loops.🛠️ Suggested change
+type NextEvent = Awaited<ReturnType<IJsEventListener['nextEventAsync']>>; const mockJsEventListener: IJsEventListener = { - nextEventAsync: () => Promise.resolve(null!), + nextEventAsync: () => new Promise<NextEvent>(() => {}), lastEvent: () => Promise.resolve(null) };
105-109: getEntryIndex ignores requested sort order.
getFilteredSortedEntriesacceptsIFilterQueryOptions, soorderis never applied and indices can be wrong when sort settings differ.🐛 Suggested fix
- async getEntryIndex(entryId: string, query?: string, options?: IIndexQueryOptions): Promise<number> { + async getEntryIndex(entryId: string, query?: string, options?: IIndexQueryOptions): Promise<number> { await delay(100); const entries = this.getFilteredSortedEntries(query, options); return entries.findIndex(e => e.id === entryId); } @@ - private getFilteredSortedEntries(query?: string, options?: IFilterQueryOptions): IEntry[] { + private getFilteredSortedEntries(query?: string, options?: IIndexQueryOptions): IEntry[] { const entries = this.getFilteredEntries(query, options); const defaultWs = writingSystems.vernacular[0].wsId; - // For getEntryIndex, we just need filtering, but we'll also sort for consistency - // Note: IFilterQueryOptions doesn't have order, so we use default sort + const sortWs = pickWs(options?.order?.writingSystem ?? 'default', defaultWs); + const ascending = options?.order?.ascending ?? true; return entries.sort((e1, e2) => { - const v1 = this.#writingSystemService.headword(e1, defaultWs); - const v2 = this.#writingSystemService.headword(e2, defaultWs); + const v1 = this.#writingSystemService.headword(e1, sortWs); + const v2 = this.#writingSystemService.headword(e2, sortWs); if (!v2) return -1; if (!v1) return 1; - const compare = v1.localeCompare(v2, defaultWs); - return compare === 0 ? e1.id.localeCompare(e2.id) : compare; + const compare = v1.localeCompare(v2, sortWs); + const result = compare === 0 ? e1.id.localeCompare(e2.id) : compare; + return ascending ? result : -result; }); }Also applies to: 232-245
🧹 Nitpick comments (10)
frontend/viewer/src/lib/components/ui/input/composable-input.svelte (1)
41-41: Consider prop ordering forid.The current order places
{id}before the spread, meaninginputProps.idwould override the component-generated id. This could break the label association at line 43 (for={id}), causing accessibility issues.If overriding
idis not an intended use case, move it after the spread:♻️ Suggested change
- <Input {id} {...mergeProps(inputProps, { class: 'grow real-input h-full px-2' })} variant="ghost" placeholder={stringPlaceholder} bind:ref={inputRef} bind:value /> + <Input {...mergeProps(inputProps, { class: 'grow real-input h-full px-2' })} {id} variant="ghost" placeholder={stringPlaceholder} bind:ref={inputRef} bind:value />If intentional (to allow consumers to provide their own id), this is fine as-is.
frontend/viewer/src/lib/components/ui/alert/alert.svelte (1)
9-9: Verify SVG icon contrast on destructive background.The destructive variant now uses
bg-destructiveas the background, but the SVG icon styling remains[&>svg]:text-destructive. Iftext-destructiveandbg-destructiveare similar colors, the icon may have poor contrast against the background.Consider updating to
[&>svg]:text-destructive-foregroundfor consistency with the text:♻️ Suggested fix
- destructive: 'bg-destructive text-destructive-foreground dark:border-destructive-foreground/50 [&>svg]:text-destructive', + destructive: 'bg-destructive text-destructive-foreground dark:border-destructive-foreground/50 [&>svg]:text-destructive-foreground',frontend/viewer/src/project/browse/EntryView.svelte (1)
60-70: Consider refetching after restore to ensure fresh server state.The
restore()function recreates the entry but doesn't refresh to get server-assigned values. AftercreateEntrycompletes, callingentryResource.refetch()would ensure the UI reflects any server-side timestamps or computed fields.♻️ Suggested improvement
async function restore() { if (!entry) return; await miniLcmApi.createEntry(entry); + await entryResource.refetch(); deleted = false; }backend/FwLite/FwLiteWeb/Routes/MiniLcmRoutes.cs (1)
198-209: Consider extracting shared query option construction.
ToIndexQueryOptions()duplicates the exemplar and sort option construction fromToQueryOptions()(lines 183-196). A private helper could reduce this repetition.♻️ Optional refactor to reduce duplication
+private (ExemplarOptions?, SortOptions) BuildCommonOptions() +{ + ExemplarOptions? exemplarOptions = string.IsNullOrEmpty(ExemplarValue) || ExemplarWritingSystem is null + ? null + : new(ExemplarValue, ExemplarWritingSystem); + var sortField = SortField ?? SortOptions.Default.Field; + var sortOptions = new SortOptions(sortField, + SortWritingSystem ?? SortOptions.Default.WritingSystem, + Ascending ?? SortOptions.Default.Ascending); + return (exemplarOptions, sortOptions); +} + public QueryOptions ToQueryOptions() { - ExemplarOptions? exemplarOptions = string.IsNullOrEmpty(ExemplarValue) || ExemplarWritingSystem is null - ? null - : new(ExemplarValue, ExemplarWritingSystem); - var sortField = SortField ?? SortOptions.Default.Field; - return new QueryOptions(new SortOptions(sortField, - SortWritingSystem ?? SortOptions.Default.WritingSystem, - Ascending ?? SortOptions.Default.Ascending), - exemplarOptions, + var (exemplarOptions, sortOptions) = BuildCommonOptions(); + return new QueryOptions(sortOptions, exemplarOptions, Count ?? QueryOptions.Default.Count, Offset ?? QueryOptions.Default.Offset, string.IsNullOrEmpty(GridifyFilter) ? null : new EntryFilter {GridifyFilter = GridifyFilter}); } public IndexQueryOptions ToIndexQueryOptions() { - ExemplarOptions? exemplarOptions = string.IsNullOrEmpty(ExemplarValue) || ExemplarWritingSystem is null - ? null - : new(ExemplarValue, ExemplarWritingSystem); - var sortField = SortField ?? SortOptions.Default.Field; - return new IndexQueryOptions(new SortOptions(sortField, - SortWritingSystem ?? SortOptions.Default.WritingSystem, - Ascending ?? SortOptions.Default.Ascending), - exemplarOptions, + var (exemplarOptions, sortOptions) = BuildCommonOptions(); + return new IndexQueryOptions(sortOptions, exemplarOptions, string.IsNullOrEmpty(GridifyFilter) ? null : new EntryFilter {GridifyFilter = GridifyFilter}); }backend/FwLite/AGENTS.md (1)
35-35: Minor: "API interfaces" is slightly redundant.Since "API" often implies "Application Programming Interface," the phrase is redundant. Consider simplifying to "APIs" or "API definitions."
✏️ Suggested edit
-The frontend viewer uses TypeScript types and API interfaces generated from .NET using **Reinforced.Typings**. These are automatically updated when you build the **FwLiteShared** project (or any project that depends on it like `FwLiteMaui` or `FwLiteWeb`). +The frontend viewer uses TypeScript types and APIs generated from .NET using **Reinforced.Typings**. These are automatically updated when you build the **FwLiteShared** project (or any project that depends on it like `FwLiteMaui` or `FwLiteWeb`).frontend/viewer/tests/entry-edit-persists.test.ts (2)
17-21: Code duplication withtest-utils.ts.This
waitForProjectViewReadyfunction duplicates the implementation fromtest-utils.ts. Consider importing from the shared utility instead to maintain consistency and reduce duplication.- async function waitForProjectViewReady(page: Page) { - await expect(page.locator('.i-mdi-loading')).toHaveCount(0, {timeout: 10000}); - await page.waitForFunction(() => document.fonts.ready); - await expect(page.locator('[data-skeleton]')).toHaveCount(0, {timeout: 10000}); - } +import {waitForProjectViewReady} from './test-utils';Then update the
beforeEachto use the imported function directly (it already supports thewaitForTestUtilsparameter).
27-30: Fixed timeout may cause flakiness.Using
page.waitForTimeout(600)is a code smell in E2E tests as it introduces arbitrary delays that may be insufficient on slower systems or unnecessarily slow on fast ones. Consider polling for a stable state instead.♻️ Suggested improvement
async function waitForEntrySaved(page: Page) { - await page.waitForTimeout(600); + // Wait for any pending debounced saves to complete await expect(page.locator('.i-mdi-loading')).toHaveCount(0, {timeout: 5000}); }If the 600ms is needed for debouncing, consider adding a comment explaining why, or poll for a specific saved state indicator.
frontend/viewer/tests/entries-list.test.ts (1)
4-7: Consider removing informal comment.The "vibe coded" comment, while honest, may not be appropriate for production test code. Consider replacing with a more descriptive comment or removing it entirely.
-/** - * This file is largely vibe coded. - * Tests for EntriesList lazy loading and virtual scrolling. - */ +/** + * Tests for EntriesList lazy loading and virtual scrolling behavior. + * Validates that entries load on initial render, during incremental scrolling, + * and after large scroll jumps. + */frontend/viewer/tests/entries-list-v2.test.ts (1)
258-262: Keep lexemeForm and citationForm in sync for updates.This makes the test data closer to real edits and avoids inconsistencies in downstream logic.
♻️ Suggested tweak
- const updated = {...entry, citationForm: {...entry.lexemeForm, seh: newLexemeForm}}; + const updated = { + ...entry, + lexemeForm: {...entry.lexemeForm, seh: newLexemeForm}, + citationForm: {...(entry.citationForm ?? {}), seh: newLexemeForm} + };backend/FwLite/LcmCrdt/Data/MiniLcmRepository.cs (1)
274-291: ROW_NUMBER() optimization is technically feasible but was already attempted.While LinqToDB does support
Sql.Ext.RowNumber().Over()and SQLite 3.25.0+ supports window functions, the code comment indicates you've already explored this approach extensively ("SQLite's ROW_NUMBER() seems to require ORDER BY in the OVER clause"). The current O(n) materialization is a pragmatic tradeoff for virtual scrolling that only loads IDs (not full entities) and is well-tested. Consider revisiting a windowed row-number approach only if this becomes a measured performance bottleneck with real-world dataset sizes.
| ### Generated .NET Types | ||
|
|
||
| This project depends on TypeScript types and API interfaces generated from .NET (via `Reinforced.Typings`). If you change .NET models or `JSInvokable` APIs, you must rebuild the backend to update these types. | ||
|
|
||
| ```bash | ||
| # From repo root | ||
| dotnet build backend/FwLite/FwLiteShared/FwLiteShared.csproj | ||
| ``` | ||
|
|
||
| The generated files are located in `src/lib/generated-types/`. |
There was a problem hiding this comment.
Verify the generated types path.
Line 27 states generated files are in src/lib/generated-types/, but based on the index.ts file path (frontend/viewer/src/lib/dotnet-types/index.ts), the correct location appears to be src/lib/dotnet-types/generated-types/.
✏️ Suggested fix
-The generated files are located in `src/lib/generated-types/`.
+The generated files are located in `src/lib/dotnet-types/generated-types/`.🧰 Tools
🪛 LanguageTool
[style] ~20-~20: This phrase is redundant (‘I’ stands for ‘interfaces’). Use simply “APIs”.
Context: ...project depends on TypeScript types and API interfaces generated from .NET (via `Reinforced.Ty...
(ACRONYM_TAUTOLOGY)
🤖 Prompt for AI Agents
In `@frontend/viewer/AGENTS.md` around lines 18 - 27, The README entry in
AGENTS.md points to the wrong generated types directory; update the path from
"src/lib/generated-types/" to the correct
"src/lib/dotnet-types/generated-types/" and sanity-check the referenced index
file (frontend/viewer/src/lib/dotnet-types/index.ts) so the doc matches the
actual location of the generated TypeScript files.
| ## Batch Loading Logic | ||
|
|
||
| ``` | ||
| Given index N: | ||
| batchNumber = floor(N / batchSize) | ||
| offset = batchNumber * batchSize | ||
| If batch not loaded and not pending: | ||
| Start fetch for entries with { offset, count: batchSize } | ||
| Store promise in pendingBatches | ||
| On resolve: cache all returned entries at their indices | ||
| Return entry from cache (or undefined if not yet loaded) | ||
| ``` |
There was a problem hiding this comment.
Add language tags to fenced code blocks.
markdownlint’s MD040 requires a language identifier; this file currently has plain fences. Add text, typescript, or svelte as appropriate to all fenced blocks.
✅ Example fix
-```
+```text
Given index N:
batchNumber = floor(N / batchSize)
offset = batchNumber * batchSize
...
-```
+```📝 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.
| ## Batch Loading Logic | |
| ``` | |
| Given index N: | |
| batchNumber = floor(N / batchSize) | |
| offset = batchNumber * batchSize | |
| If batch not loaded and not pending: | |
| Start fetch for entries with { offset, count: batchSize } | |
| Store promise in pendingBatches | |
| On resolve: cache all returned entries at their indices | |
| Return entry from cache (or undefined if not yet loaded) | |
| ``` | |
| ## Batch Loading Logic | |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
100-100: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@frontend/viewer/src/lib/services/ENTRY_LOADER_PLAN.md` around lines 98 - 111,
The fenced code block under "Batch Loading Logic" lacks a language identifier
which violates markdownlint MD040; update the triple-backtick fence to include
an appropriate language tag (for example change ``` to ```text) so the block
becomes ```text ...```; ensure any other fenced blocks in ENTRY_LOADER_PLAN.md
also get suitable tags (e.g., typescript or svelte where applicable) to satisfy
the linter.
| ### API Surface of `EntryLoaderService` | ||
|
|
||
| ```typescript | ||
| // Construction | ||
| constructor(deps: { | ||
| miniLcmApi: () => IMiniLcmApi | undefined; | ||
| search: () => string; | ||
| sort: () => SortConfig | undefined; | ||
| gridifyFilter: () => string | undefined; | ||
| }) | ||
|
|
||
| // Reactive state (read-only) | ||
| readonly totalCount: number | undefined; | ||
| readonly loading: boolean; | ||
| readonly error: Error | undefined; | ||
|
|
||
| // Core methods | ||
| getEntryByIndex(index: number): IEntry | undefined; // sync, from cache | ||
| loadEntryByIndex(index: number): Promise<IEntry>; // async, fetches batch if needed | ||
| loadCount(): Promise<void>; // fetches total count | ||
|
|
||
| // Event handling | ||
| getIndexById(id: string): number | undefined; // from incremental id→index map | ||
| removeEntryById(id: string): void; // handles delete | ||
| updateEntry(entry: IEntry): void; // handles update | ||
| reset(): void; // full reset | ||
|
|
||
| // V2 additions | ||
| getVersion(index: number): number; // for VList key invalidation | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "class EntryLoaderService|getCachedEntryByIndex|getOrLoadEntryByIndex|loadInitialCount|getEntryByIndex|loadEntryByIndex|loadCount" frontend/viewer/src/lib/services/entry-loader-service.svelte.tsRepository: sillsdev/languageforge-lexbox
Length of output: 339
Update the documented API surface to match the actual EntryLoaderService method names.
The documented methods do not match the implementation. The actual method names are:
getCachedEntryByIndex(notgetEntryByIndex)getOrLoadEntryByIndex(notloadEntryByIndex)loadInitialCount(notloadCount)
Correct the API documentation
-getEntryByIndex(index: number): IEntry | undefined; // sync, from cache
-loadEntryByIndex(index: number): Promise<IEntry>; // async, fetches batch if needed
-loadCount(): Promise<void>; // fetches total count
+getCachedEntryByIndex(index: number): IEntry | undefined; // sync, from cache
+getOrLoadEntryByIndex(index: number): Promise<IEntry | undefined>; // async, fetches batch if needed
+loadInitialCount(): Promise<void>; // fetches total count🤖 Prompt for AI Agents
In `@frontend/viewer/src/lib/services/ENTRY_LOADER_PLAN.md` around lines 168 -
197, Update the API surface in ENTRY_LOADER_PLAN.md so the documented method
names match the implementation: replace getEntryByIndex with
getCachedEntryByIndex, replace loadEntryByIndex with getOrLoadEntryByIndex, and
replace loadCount with loadInitialCount; ensure the signatures/comments remain
correct (sync vs async, batch-fetch behavior) and remove or mark the old names
to avoid confusion.
| async updateEntry(entry: IEntry): Promise<void> { | ||
| const generation = this.#generation; | ||
| if (await this.tryOptimizeUpdateEntryEvent(entry)) return; | ||
| if (generation !== this.#generation) return; // outdated event => abort | ||
| await this.quietReset(); | ||
| } | ||
|
|
||
| /** | ||
| * The more trivial and performant checks we can do to verify if the event is relevant | ||
| * to our current state. | ||
| */ | ||
| private async tryOptimizeUpdateEntryEvent(entry: IEntry): Promise<boolean> { | ||
| const cachedIndex = this.#idToIndex.get(entry.id); | ||
| if (cachedIndex !== undefined) { | ||
| // we've seen it locally, so it wasn't an add event | ||
| if (cachedIndex < 0) { | ||
| // not relevant for our current filter => ignore | ||
| return true; | ||
| } | ||
| const batchIndex = Math.floor(cachedIndex / this.batchSize); | ||
| if (!this.#loadedBatches.has(batchIndex)) { | ||
| // it's not new and we haven't loaded it yet => ignore | ||
| return true; | ||
| } | ||
| } else { | ||
| const entryIndex = await this.getOrLoadEntryIndex(entry.id); | ||
| if (entryIndex < 0) { | ||
| // not relevant for our current filter => ignore | ||
| return true; |
There was a problem hiding this comment.
Don’t ignore update events for cached “not found” entries.
Line 192‑195 returns early when cachedIndex < 0, but updates can make a previously filtered‑out entry match; skipping the quiet reset leaves the list stale.
🛠️ Suggested fix
- if (cachedIndex < 0) {
- // not relevant for our current filter => ignore
- return true;
- }
+ if (cachedIndex < 0) {
+ // Previously filtered out; updates can make it relevant.
+ return false; // force quietReset
+ }🤖 Prompt for AI Agents
In `@frontend/viewer/src/lib/services/entry-loader-service.svelte.ts` around lines
177 - 205, The current optimization wrongly ignores update events for cached
entries with cachedIndex < 0; instead of returning true (which skips
quietReset), treat a cached "not found" as potentially becoming relevant by not
short‑circuiting. In tryOptimizeUpdateEntryEvent, remove or change the early
return for the cachedIndex < 0 branch so it does not return true — let the
function fall through (return false) so updateEntry will call quietReset when
needed; reference symbols: tryOptimizeUpdateEntryEvent, cachedIndex,
updateEntry, quietReset.
| #. EntryView label for the current dictionary object when FieldWorks Classic is shown | ||
| #: src/project/browse/EntryView.svelte | ||
| msgid "entry" | ||
| msgstr "" | ||
|
|
There was a problem hiding this comment.
Provide French translations for the new UI strings.
These msgid entries still have empty msgstr values, so they’ll fall back to English in the UI. Please add translations for “entry”, “word”, “Filter”, “Restore”, and “This {0} was deleted”.
Also applies to: 786-789, 1418-1422, 1702-1706, 1901-1905
🤖 Prompt for AI Agents
In `@frontend/viewer/src/locales/fr.po` around lines 626 - 630, Fill the empty
msgstr entries for the listed msgid strings with French translations: replace
msgstr "" for "entry" with "entrée" (or "article" if context prefers), for
"word" use "mot", for "Filter" use "Filtrer" (or "Filtre" depending on UI
verb/noun context), for "Restore" use "Restaurer", and for "This {0} was
deleted" use "Ce {0} a été supprimé"; apply these same translations to the other
occurrences referenced (lines ~786-789, 1418-1422, 1702-1706, 1901-1905) so no
msgid remains with an empty msgstr.
| {#snippet children(state)} | ||
| {@const start = Date.now()} | ||
| {#if state.loading} | ||
| <div class="flex items-center gap-2 text-muted-foreground italic"> | ||
| <Loading /> | ||
| Waiting {args.delay}ms before loading and {simulatedLoadTime}ms to simulate load... | ||
| </div> | ||
| {:else if state.error} | ||
| <div class="text-destructive">Error: {state.error}</div> | ||
| {:else} | ||
| {@const end = Date.now()} | ||
| <div class="text-primary font-bold">Loaded: {state.current} (in {end - start}ms)</div> | ||
| {/if} |
There was a problem hiding this comment.
Timing measurement will always show ~0ms.
The start timestamp is captured inside the snippet which re-renders when state changes. When the content displays (state.current), start is re-captured at that moment, so end - start will always be nearly 0ms rather than reflecting the actual load time.
💡 Suggested fix to measure actual load time
Move the timing capture outside the snippet or use a different approach:
<Story name="Uncached (Delayed)">
{`#snippet` template(args)}
{`@const` simulatedLoadTime = 500}
+ {`@const` start = Date.now()}
<div class="p-4 border rounded bg-card">
<Delayed
load={async () => {
await delay(simulatedLoadTime);
return '😎🎈';
}}
delay={args.delay}
>
{`#snippet` children(state)}
- {`@const` start = Date.now()}
{`#if` state.loading}
<!-- ... -->
{:else if state.error}
<!-- ... -->
{:else}
{`@const` end = Date.now()}
<div class="text-primary font-bold">Loaded: {state.current} (in {end - start}ms)</div>
{/if}
{/snippet}
</Delayed>
</div>
{/snippet}
</Story>🤖 Prompt for AI Agents
In `@frontend/viewer/src/stories/primitives/delayed.stories.svelte` around lines
42 - 54, The timing is captured inside the {`#snippet` children(state)} block so
`@const start` is recomputed on every render and makes `end - start` ~0; fix by
capturing the start time when the load begins (e.g., set a `start` timestamp on
the `state` object or a top-level variable when you initiate the delay) and then
read that persisted `state.start` inside the snippet to compute `end -
state.start`; update any references to `start`/`end` accordingly (keep `end`
local when displaying but use the persisted `state.start` for the subtraction).
No description provided.