Skip to content

feat: import a dictionary from inside the app - #32

Merged
widgetii merged 2 commits into
mainfrom
feat/m6-import-ui
Oct 18, 2026
Merged

widgetii merged 2 commits into
mainfrom
feat/m6-import-ui

Conversation

@widgetii

Copy link
Copy Markdown
Member

M6's last gap: a reader still needed the command line to install a dictionary. This is the dictionaries screen and the import that runs from it — driven and watched in the real app rather than reasoned about.

A long import needs three things

Progress they can believe. The dictionary's own declared name, which file of the set, how many entries so far.

A Stop that leaves the work intact — which shouldContinue already gave the importer, and which the screen says plainly: stopping keeps what has been imported so far.

Resuming. The container is written straight into the dictionaries directory under the name it will finally have, so starting the same import again finds the job row inside it and carries on. Writing to a temporary file and moving it on success would throw away a minute of work every time the reader changed their mind.

Verified by stopping a 3.4-million-card import and starting it again: it resumed at file 3 of 3 with 2,662,043 entries already written, not at zero.

A half-imported container is unfinished, not unreadable

Metadata is written when an import finishes, so one without any was being listed as a file that could not be read. That is both wrong and useless — the answer is to import it again. The screen now says so, and says what to do.

Attribution reaches the reader here

Licence and attribution are read from each container's own meta and shown per dictionary. That is a CC BY-SA obligation rather than a nicety, and reading it from the file means it is correct for a dictionary the reader imported as well as for the bundled one. The three permissions — definitions, readings, word list — are three switches, with the paragraph explaining why they are separate.

An imported dictionary reaches a text already open

The services are rebuilt rather than mutated: opening containers costs milliseconds, and the caches it discards are caches of answers that have just changed. The reader view is keyed on the dictionaries as well as the text, so the model holding those caches is rebuilt with them.

Three defects the running app found

Every one of these passed every test and looked fine in the code.

A list row is as tall as it believes its content to be, and it does not believe a progress view with a label. The dictionary's name and the file-and-entry count were clipped away above the bar, leaving an unlabelled line creeping across the screen — exactly the case where a reader needs telling what is happening. The banner now sits above the list rather than in it.

A section footer gets one line and the rest is cut off, which for the paragraph explaining the three switches was all of it.

A sheet on macOS sizes itself to its content, and a List has no height to offer — the first version opened as a title bar with nothing underneath.

And one of mine, found while fixing those: the status view's .task(id:) was keyed on the import phase, which is the progress, so it re-ran ten times a second. The reload now happens once, from the model, when the import stops for any reason.

What it looks like

Dictionaries
┌ Importing ─────────────────────────────────┐
│ 大БКРС - 250920                            │
│ File 3 of 3, 2 662 043 entries             │
│ ████████████████████░░░░░░░                │
│ [Stop]  Stopping keeps what has been       │
│         imported so far.                   │
└────────────────────────────────────────────┘
Installed
  CC-CEDICT                          107,619
  zh-Hans → en
  Definitions ● Readings ● Word list ●
  › Licence and attribution
  大БКРС - 250920                  3,445,226
  zh-Hans → ru
  Definitions ● Readings ● Word list ○
  › Licence and attribution   [Remove…]

And the detail panel, with both dictionaries grouped under one reading and each labelled:

瘦弱  🔊
shòuruò
  shòuruò
  CC-CEDICT
    1. thin and weak
  大БКРС - 250920
    1. худой, худосочный, чахлый; хилый, слабый, тощий…

5 new tests for the import model. Full suite green, lint clean, no warnings under -warnings-as-errors.

🤖 Generated with Claude Code

M6's last gap: a reader still needed the command line. This is the
dictionaries screen and the import that runs from it, driven and watched in
the real app rather than reasoned about.

**The import is long, so three things are not optional.** Progress the reader
can believe — the dictionary's own name, which file of the set, how many
entries so far. A Stop that leaves the work intact, which `shouldContinue`
already gave the importer and which says so on screen: *stopping keeps what
has been imported so far*. And resuming: the container is written straight
into the dictionaries directory under the name it will finally have, so
starting the same import again finds the job row inside it and carries on.
Verified by stopping a 3.4-million-card import and starting it again — it
resumed at file 3 of 3 with 2,662,043 entries already written rather than at
zero.

**A half-imported container is unfinished, not unreadable.** Metadata is
written when an import finishes, so one without any was being listed as a
file that could not be read — which is both wrong and useless, since the
answer is to import it again. It says so now, and how to fix it.

**The screen is where attribution reaches the reader.** Licence and
attribution are read from each container's own `meta` and shown per
dictionary, which is a CC BY-SA obligation rather than a nicety, and is
correct for a dictionary the reader imported as well as the bundled one. The
three permissions are three switches, with the paragraph that explains why.

**An imported dictionary reaches a text that is already open.** The services
are rebuilt rather than mutated — opening containers costs milliseconds, and
the caches it discards are caches of answers that have just changed — and the
reader view is keyed on the dictionaries as well as the text, so the model
holding those caches is rebuilt with them.

### Three defects the running app found

Every one of these passed every test and looked fine in code.

**A list row is as tall as it believes its content to be, and it does not
believe a progress view with a label.** The dictionary's name and the
file-and-entry count were clipped away above the bar, leaving an unlabelled
line creeping across the screen — which is exactly the case where a reader
needs to be told what is happening. The banner now sits above the list rather
than in it.

**A section footer gets one line and the rest is cut off**, which for the
paragraph explaining the three switches was all of it. It is an ordinary row
now.

**A sheet on macOS sizes itself to its content, and a `List` has no height to
offer**, so the first version opened as a title bar with nothing underneath.

And one of mine, caught while fixing those: the status view's `.task(id:)` was
keyed on the import phase, which *is* the progress — so it re-ran ten times a
second. The reload now happens once, from the model, when the import stops for
any reason.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@widgetii
widgetii enabled auto-merge (squash) October 18, 2026 18:30
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Import and manage dictionaries from inside the app

✨ Enhancement 🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Add an in-app dictionary screen with import progress, stopping, permissions, removal, and
 attribution.
• Resume interrupted imports and distinguish unfinished containers from unreadable files.
• Reload dictionary services so changes reach open texts; add import-model tests.
Diagram

graph TD
  Sidebar["Library sidebar"] --> Screen["Dictionaries screen"] --> Model["Import model"] --> Importer["DSL importer"] --> Container[("Dictionary container")] --> Catalogue["Dictionary catalogue"] --> Services["App services"] --> Reader["Reader screen"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Resumable staging file with atomic promotion
  • ➕ Keeps incomplete containers out of the installed-dictionary directory.
  • ➖ Requires stable staging paths, recovery rules, and promotion after completion.
  • ➖ Adds lifecycle complexity despite the existing import job row identifying unfinished work.

Recommendation: Writing to the final path is a reasonable fit for the existing resumable importer, provided the catalogue continues to distinguish unfinished containers. Consolidate reload ownership before merging: the model calls onFinished at completion, while the screen also calls onChange from a terminal-phase task, causing a second service rebuild.

Files changed (10) +881 / -11

Enhancement (7) +711 / -10
DSLImporter.swiftExpose stable import destination and display names +29/-0

Expose stable import destination and display names

• Adds helpers that derive the slug and display name from the same metadata used by the finished container. The UI can choose a resumable destination and show the dictionary’s declared name before import completes.

Sources/HanReaderDictionaryImport/DSLImporter.swift

DictionaryCatalogue.swiftClassify unfinished imports and expose reading syllables +79/-10

Classify unfinished imports and expose reading syllables

• Separates containers with an active import job from unreadable files. Adds a way to collect syllable bases from reading-enabled dictionaries for cross-dictionary reading matching.

Sources/HanReaderPersistence/DictionaryCatalogue.swift

HanReaderRootView.swiftPresent dictionary management and refresh open readers +41/-0

Present dictionary management and refresh open readers

• Connects the sidebar to the new sheet and rebuilds app services after dictionary changes. A dictionary generation counter prompts an already-open reader to rebuild its cached model.

Sources/HanReaderUI/HanReaderRootView.swift

DictionariesScreen.swiftAdd the dictionary management sheet +354/-0

Add the dictionary management sheet

• Shows installed dictionaries, per-dictionary attribution and permissions, unfinished imports, and unreadable files. Provides import progress and stop controls, removal confirmation, and layout that gives the macOS sheet and progress banner usable space.

Sources/HanReaderUI/Screens/DictionariesScreen.swift

LibrarySidebar.swiftAdd a Dictionaries toolbar action +6/-0

Add a Dictionaries toolbar action

• Adds the sidebar action that opens dictionary management.

Sources/HanReaderUI/Screens/LibrarySidebar.swift

AppServices.swiftRebuild immutable dictionary services on reload +29/-0

Rebuild immutable dictionary services on reload

• Extracts shared service construction and adds a reload path that retains the library repository while reopening the catalogue, lookup service, and segmenter.

Sources/HanReaderUI/Services/AppServices.swift

DictionaryImportModel.swiftDrive resumable DSL imports from observable UI state +173/-0

Drive resumable DSL imports from observable UI state

• Discovers file sets, writes to stable destination paths, and reports names, progress, completion, and failures. A thread-safe stop flag requests a graceful stop without discarding committed work.

Sources/HanReaderUI/Services/DictionaryImportModel.swift

Bug fix (2) +30 / -1
DictionaryContainer.swiftDetect an unfinished container import +14/-0

Detect an unfinished container import

• Adds a query for a remaining import job row, allowing catalogue scans to identify imports that can be resumed.

Sources/HanReaderPersistence/DictionaryContainer.swift

ReaderScreen.swiftInclude dictionary changes in reader model identity +16/-1

Include dictionary changes in reader model identity

• Keys the reader’s model-building task on both the selected text and dictionary generation, replacing cached readings and definitions when services change.

Sources/HanReaderUI/Screens/ReaderScreen.swift

Tests (1) +140 / -0
DictionaryImportModelTests.swiftTest import-model outcomes and destination +140/-0

Test import-model outcomes and destination

• Adds five tests for successful import reporting, declared names, destination and entry count, invalid input reaching an end state, and reset behavior.

Tests/HanReaderUITests/DictionaryImportModelTests.swift

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Multi-file imports fail in the sandbox 🐞 Bug ≡ Correctness
Description
DictionaryImportModel.start acquires security-scoped access only for the file selected in the
picker. For a set discovered through includes or numbered siblings, discovery and import also read
other file URLs, which may not be accessible under that file-only grant.
Code

Sources/HanReaderUI/Services/DictionaryImportModel.swift[R80-87]

+                let scoped = url.startAccessingSecurityScopedResource()
+                defer {
+                    if scoped {
+                        url.stopAccessingSecurityScopedResource()
+                    }
+                }
+
+                let fileSet = try DSLFileSet.discover(from: url)
Evidence
The scope wraps only url; DSLFileSet.discover can add other URLs, and the importer subsequently
opens each discovered source.

Sources/HanReaderUI/Services/DictionaryImportModel.swift[77-109]
Sources/HanReaderCore/Format/DSLFileSet.swift[68-100]
Sources/HanReaderCore/Format/DSLFileSet.swift[125-153]
Sources/HanReaderDictionaryImport/DSLImporter.swift[163-169]
Sources/HanReaderDictionaryImport/DSLImporter.swift[367-379]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The picker grants access to the selected file, but a discovered DSL set can require sibling or included files that the import has not been granted permission to read.
## Fix Focus Areas
- Sources/HanReaderUI/Services/DictionaryImportModel.swift[77-109]
- Sources/HanReaderCore/Format/DSLFileSet.swift[68-100]
- Sources/HanReaderCore/Format/DSLFileSet.swift[125-153]
## Recommended Fix
Arrange an appropriate directory or per-volume access grant before discovering and reading the set. Keep all required grants active until the detached import completes, and report missing access rather than compiling an incomplete set.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Quick switch changes overwrite each other ✓ Resolved
Description
update saves a complete DictionaryPreference assembled from the row’s captured entry, and each
switch launches an independent task. If a reader changes two permissions before the first save
refreshes the row, both saves start with the old values and the later save restores the other
permission.
Code

Sources/HanReaderUI/Screens/DictionariesScreen.swift[R240-247]

+            permission("Definitions", isOn: entry.providesGlosses) {
+                await update(entry) { $0.with(glosses: !entry.providesGlosses) }
+            }
+            permission("Readings", isOn: entry.providesReadings) {
+                await update(entry) { $0.with(readings: !entry.providesReadings) }
+            }
+            permission("Word list", isOn: entry.providesLexicon) {
+                await update(entry) { $0.with(lexicon: !entry.providesLexicon) }
Evidence
The switch task captures the old row, update constructs all fields from that row, and the
repository upsert writes all permission columns.

Sources/HanReaderUI/Screens/DictionariesScreen.swift[165-181]
Sources/HanReaderUI/Screens/DictionariesScreen.swift[240-247]
Sources/HanReaderUI/Screens/DictionariesScreen.swift[274-285]
Sources/HanReaderPersistence/LibraryRepository.swift[382-406]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Independent switch tasks save whole preference snapshots, so rapid changes to different permissions can overwrite one another.
## Fix Focus Areas
- Sources/HanReaderUI/Screens/DictionariesScreen.swift[165-181]
- Sources/HanReaderUI/Screens/DictionariesScreen.swift[240-247]
- Sources/HanReaderUI/Screens/DictionariesScreen.swift[274-285]
## Recommended Fix
Serialize updates for a dictionary and derive each change from its latest preference, or update only the affected column atomically. Reflect pending switch changes without relying on a later catalogue reload to preserve them.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Failed imports leave the list out of date ✓ Resolved
Description
DictionaryImportModel.start calls onFinished after normal completion or stopping, but its
catch changes only the phase to .failed. If a file read or database operation throws after
creating or writing a partial container, the catalogue is not reloaded to show that file until
another change or launch.
Code

Sources/HanReaderUI/Services/DictionaryImportModel.swift[R117-119]

+            } catch {
+                phase = .failed(String(describing: error))
+            }
Evidence
The writer opens the destination before processing files; later file reading can throw. Only the
successful path calls onFinished, and the status view does not reload for .failed.

Sources/HanReaderUI/Services/DictionaryImportModel.swift[102-120]
Sources/HanReaderDictionaryImport/DSLImporter.swift[163-196]
Sources/HanReaderDictionaryImport/DSLImporter.swift[367-388]
Sources/HanReaderUI/Screens/DictionariesScreen.swift[341-350]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Failures after import work has begun can leave a partial container on disk, but the failure path never refreshes the catalogue.
## Fix Focus Areas
- Sources/HanReaderUI/Services/DictionaryImportModel.swift[102-120]
- Sources/HanReaderUI/Screens/DictionariesScreen.swift[90-98]
## Recommended Fix
Invoke the catalogue refresh callback on all terminal paths, including thrown errors, while preserving the failure message. Ensure that the callback runs once per attempt.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Removed dictionaries remain in open texts ✓ Resolved
Description
remove deletes the container before calling the separately throwing forgetDictionary, and it
reloads services only after both operations succeed. If forgetting the preference fails, the file is
gone but the active reader retains its old dictionary services; retrying removal fails on the
already-missing file before it can reach the reload.
Code

Sources/HanReaderUI/Screens/DictionariesScreen.swift[R187-191]

+            try FileManager.default.removeItem(at: entry.file)
+            // And the reader's opinion about it, since they asked for it to
+            // be gone rather than for it to be temporarily unavailable.
+            try await services.library.forgetDictionary(entry.slug)
+            await onChange()
Evidence
File deletion, preference deletion, and reload occur sequentially in one throwing block; only reload
replaces the services and advances the generation used by the reader.

Sources/HanReaderUI/Screens/DictionariesScreen.swift[184-195]
Sources/HanReaderPersistence/LibraryRepository.swift[424-431]
Sources/HanReaderUI/HanReaderRootView.swift[79-85]
Sources/HanReaderUI/Screens/ReaderScreen.swift[45-66]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A preference-deletion error after the file has been removed skips the services reload and prevents a normal retry.
## Fix Focus Areas
- Sources/HanReaderUI/Screens/DictionariesScreen.swift[184-195]
## Recommended Fix
Track whether the file was removed and reload dictionaries even if preference cleanup fails. Handle an already-missing file on retry, and report preference-cleanup failure independently.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View medium (5)
5. Early stops are listed as unreadable ✓ Resolved
Description
Accumulator.add treats a metadata-less container as unfinished only when hasUnfinishedImport
finds a job row. Stopping before the importer’s first batch leaves a created container without
metadata or a job row, so the screen instead tells the reader the file is not readable, even though
importing it again can recover.
Code

Sources/HanReaderPersistence/DictionaryCatalogue.swift[R254-257]

+                    if (try? container.hasUnfinishedImport()) == true {
+                        unfinished.append(url)
+                    } else {
+                        unreadable.append(UnreadableDictionary(
Evidence
Opening creates the database without a job row; the early stop check returns before a batch commit
or end-of-file checkpoint, and the new catalogue branch requires a row to classify it as unfinished.

Sources/HanReaderPersistence/DictionaryCatalogue.swift[246-262]
Sources/HanReaderPersistence/DictionaryContainer.swift[321-325]
Sources/HanReaderPersistence/DictionaryWriter.swift[208-229]
Sources/HanReaderDictionaryImport/DSLImporter.swift[382-390]
Sources/HanReaderDictionaryImport/DSLImporter.swift[432-447]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
An import stopped before its first checkpoint has no job row, so the new unfinished classification mistakes its container for an unreadable file.
## Fix Focus Areas
- Sources/HanReaderPersistence/DictionaryCatalogue.swift[246-262]
- Sources/HanReaderPersistence/DictionaryContainer.swift[321-326]
- Sources/HanReaderPersistence/DictionaryWriter.swift[208-229]
## Recommended Fix
Create an identifiable import job when opening a new destination, before processing its first card, or reliably identify and classify the resulting metadata-less container as an unfinished import.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Stopped imports still show as unreadable ✓ Resolved
Description
The Accumulator.add branch reaches hasUnfinishedImport() only after
DictionaryContainer(contentsOf:) has opened the file read-only and metadata() has read it. A
stopped import is still in WAL mode, because journal_mode = DELETE is only set in finish, so
when the writer closes and its -wal/-shm files are removed, the read-only open fails and the generic
catch files it under "Not readable" with a raw SQLite error, which means the new "Unfinished"
section is likely never shown after a Stop.
Code

Sources/HanReaderPersistence/DictionaryCatalogue.swift[R246-255]

+                let container = try DictionaryContainer(contentsOf: url)
+                guard let metadata = try container.metadata() else {
+                    // A container writes its metadata when the import
+                    // finishes, so one without any is either an import that
+                    // stopped — which it says by still carrying its job row
+                    // — or a file that is not a dictionary at all. The
+                    // difference matters: one of them is fixed by importing
+                    // it again, and the other never will be.
+                    if (try? container.hasUnfinishedImport()) == true {
+                        unfinished.append(url)
Evidence
DictionaryWriter.open sets PRAGMA journal_mode = WAL and only finish switches it back. Its own
comment says a WAL-header container opened the way the app opens it fails with "unable to open
database file". DictionaryContainer.init opens with readonly = true and runs a pragma in
prepareDatabase. Any throw there or in metadata() lands in the catch, which appends to
unreadable. No test covers the unfinished path.

Sources/HanReaderPersistence/DictionaryWriter.swift[201-207]
Sources/HanReaderPersistence/DictionaryWriter.swift[450-470]
Sources/HanReaderPersistence/DictionaryContainer.swift[18-25]
Sources/HanReaderPersistence/DictionaryCatalogue.swift[281-286]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A stopped import leaves its container in WAL journal mode. The catalogue opens every container read-only, which fails for a WAL file that has no -shm, so the file goes to `unreadable` before `hasUnfinishedImport()` is ever asked.
## Fix Focus Areas
- Sources/HanReaderPersistence/DictionaryCatalogue.swift[239-287]
- Sources/HanReaderPersistence/DictionaryContainer.swift[321-326]
- Sources/HanReaderPersistence/DictionaryWriter.swift[450-470]
## Recommended Fix
Pick one: (a) when an import stops, have the writer checkpoint and switch to `journal_mode = DELETE` too, not only in `finish`; or (b) make `hasUnfinishedImport` a static helper that opens the file read-write (or with `immutable=1`) and call it from the `catch` branch before reporting the file as unreadable. Add a test that stops an import, reads the catalogue, and checks the file is in `unfinished`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Each finished import reloads everything twice ✓ Resolved
Description
importStatus retains a .task(id: importModel.phase) that calls onChange() for finished and
cancelled imports, duplicating the callback already invoked by DictionaryImportModel.start. On
either terminal phase, the duplicate reload reopens containers, increments dictionaryGeneration,
and rebuilds the open reader’s model twice, while the phase-keyed task also re-runs on progress
updates.
Code

Sources/HanReaderUI/Screens/DictionariesScreen.swift[R341-349]

+        .task(id: importModel.phase) {
+            // The list is built from the catalogue, so it only shows a new
+            // dictionary once the catalogue has been read again.
+            if case .finished = importModel.phase {
+                await onChange()
+            }
+            if case .cancelled = importModel.phase {
+                await onChange()
+            }
Evidence
DictionariesScreen passes onChange to importModel.start, and the model invokes that callback
after setting a finished or cancelled phase. The importStatus task observes the same terminal
phase and calls onChange again; each reload increments dictionaryGeneration, which ReaderScreen
uses to trigger its model rebuild.

Sources/HanReaderUI/Screens/DictionariesScreen.swift[92-92]
Sources/HanReaderUI/Services/DictionaryImportModel.swift[111-116]
Sources/HanReaderUI/HanReaderRootView.swift[79-88]
Sources/HanReaderUI/Screens/ReaderScreen.swift[48-48]
Sources/HanReaderUI/Screens/DictionariesScreen.swift[90-93]
Sources/HanReaderUI/Screens/DictionariesScreen.swift[341-350]
Sources/HanReaderUI/HanReaderRootView.swift[79-85]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Import completion or cancellation reloads dictionaries twice: once through the model callback and again through the phase-keyed `importStatus` task.
## Fix Focus Areas
- Sources/HanReaderUI/Screens/DictionariesScreen.swift[341-350]
- Sources/HanReaderUI/Screens/DictionariesScreen.swift[90-93]
- Sources/HanReaderUI/Services/DictionaryImportModel.swift[111-116]
## Recommended Fix
Remove the `.task(id: importModel.phase) { ... }` modifier from `importStatus`. Keep the model’s completion callback as the single reload path for both finished and cancelled imports.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. Closing the sheet orphans a running import ✓ Resolved
Description
DictionariesScreen owns DictionaryImportModel as sheet-local @State, so Done discards the
model and its Stop control while the unstructured task and detached importer continue writing.
Reopening the sheet creates an idle model that can start the same import again, allowing a second
DictionaryWriter to resume from the shared checkpoint and delete or rewrite rows while the first
writer is still appending to the file.
Code

Sources/HanReaderUI/Screens/DictionariesScreen.swift[R24-28]

+    init(services: AppServices, onChange: @escaping () async -> Void) {
+        self.services = services
+        self.onChange = onChange
+        _importModel = State(initialValue: DictionaryImportModel(services: services))
+    }
Evidence
DictionariesScreen creates the model as @State, and Done dismisses the sheet without stopping
the import; only the Import button is disabled during work. DictionaryImportModel.start launches
an unstructured task, parsing runs in a detached task, and isRunning belongs to each model
instance, so a reopened sheet does not reflect the existing job. On resume, DictionaryWriter.open
deletes entries at or beyond the checkpoint, while append commits between transactions, allowing
two writers targeting the same file to interleave.

Sources/HanReaderUI/Screens/DictionariesScreen.swift[76-84]
Sources/HanReaderUI/Services/DictionaryImportModel.swift[63-109]
Sources/HanReaderPersistence/DictionaryWriter.swift[290-301]
Sources/HanReaderUI/Screens/DictionariesScreen.swift[75-98]
Sources/HanReaderUI/Services/DictionaryImportModel.swift[39-48]
Sources/HanReaderUI/Services/DictionaryImportModel.swift[102-116]
Sources/HanReaderUI/HanReaderRootView.swift[233-237]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Dismissing the management sheet leaves its import running without an accessible Stop control. Reopening the sheet can start a second import against the same destination.
## Fix Focus Areas
- Sources/HanReaderUI/Screens/DictionariesScreen.swift[15-28]
- Sources/HanReaderUI/Screens/DictionariesScreen.swift[75-98]
- Sources/HanReaderUI/HanReaderRootView.swift[233-237]
- Sources/HanReaderUI/Services/DictionaryImportModel.swift[39-48]
- Sources/HanReaderUI/Services/DictionaryImportModel.swift[102-116]
## Recommended Fix
Prefer owning `DictionaryImportModel` in `LibraryAndReader` or `HanReaderLaunch` and passing it into `DictionariesScreen`, so reopening the sheet shows the active import and its Stop button. Alternatively, prevent dismissal while `isRunning`, or stop the import before dismissal and wait for a checkpoint. In either case, prevent a second job from targeting an active destination.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


9. Re-importing wipes an installed dictionary ✓ Resolved
Description
DictionaryImportModel.start derives the destination solely from the declared name’s slug, without
checking whether the path belongs to an installed dictionary or a different source. When an import
reuses that slug and has no matching resumable job, DictionaryWriter.open clears the existing
entries, lexemes, readings and metadata; stopping or failing afterward can leave a finished
dictionary unusable or discard entries from an unfinished import.
Code

Sources/HanReaderUI/Services/DictionaryImportModel.swift[R87-92]

+                let fileSet = try DSLFileSet.discover(from: url)
+                let destination = services.locations.dictionaryDirectory
+                    .appendingPathComponent(
+                        "\(DSLImporter.slug(for: fileSet))."
+                            + DictionaryCatalogue.fileExtension,
+                    )
Evidence
The UI constructs the destination from DSLImporter.slug, which uses the declared display name, so
selecting the same set again or a different set with the same normalized name reaches the same file.
When there is no resumable checkpoint and existing entries are present, the writer deletes data from
entry, lexeme, charReading, syllableBase and meta. The Import button is disabled only
while an import is running, not when the destination already exists.

Sources/HanReaderDictionaryImport/DSLImporter.swift[536-545]
Sources/HanReaderPersistence/DictionaryWriter.swift[275-288]
Sources/HanReaderUI/Screens/DictionariesScreen.swift[79-84]
Sources/HanReaderUI/Services/DictionaryImportModel.swift[87-100]
Sources/HanReaderDictionaryImport/DSLImporter.swift[532-550]
Sources/HanReaderDictionaryImport/DSLImporter.swift[560-575]
Sources/HanReaderPersistence/DictionaryWriter.swift[208-229]
Sources/HanReaderPersistence/DictionaryWriter.swift[267-295]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
An import can target an existing dictionary file solely because its declared name produces the same slug. Without distinguishing a matching unfinished import from an installed dictionary or another source, starting the import can clear existing data before the new import succeeds.
## Fix Focus Areas
- Sources/HanReaderUI/Services/DictionaryImportModel.swift[87-101]
- Sources/HanReaderUI/Screens/DictionariesScreen.swift[86-99]
- Sources/HanReaderPersistence/DictionaryWriter.swift[208-229]
- Sources/HanReaderPersistence/DictionaryWriter.swift[267-295]
## Recommended Fix
Before importing to an existing slug path, determine whether it belongs to the selected source and whether it appears in `services.catalogue.installed` or `catalogue.unfinished`. Allow resuming a matching unfinished import in place. Otherwise, require an explicit replacement decision or allocate a distinct destination; for replacement, consider importing into a sibling temporary file and replacing the existing dictionary only after success, rather than clearing it at the start.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can move or hide resolved findings to focus on what still needs action

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread Sources/HanReaderUI/Services/DictionaryImportModel.swift
Comment thread Sources/HanReaderUI/Screens/DictionariesScreen.swift Outdated
Comment thread Sources/HanReaderUI/Services/DictionaryImportModel.swift
Comment thread Sources/HanReaderUI/Screens/DictionariesScreen.swift Outdated
Comment thread Sources/HanReaderPersistence/DictionaryCatalogue.swift Outdated
Comment thread Sources/HanReaderPersistence/DictionaryCatalogue.swift Outdated
Comment thread Sources/HanReaderUI/Screens/DictionariesScreen.swift Outdated
Comment thread Sources/HanReaderUI/Screens/DictionariesScreen.swift Outdated
Comment thread Sources/HanReaderUI/Services/DictionaryImportModel.swift Outdated
Nine findings from review, and one of them subsumes three others.

**An import in progress now has a name of its own.** It wrote straight to
`<slug>.hanreaderdict`, which is the file the reader is already using — and
`DictionaryWriter.open` clears a destination it cannot resume into, so
re-importing a dictionary destroyed the copy they had the moment it started,
and stopping or failing afterwards left them with neither. It writes to
`<slug>.hanreaderdict-import` and is moved into place in one step when it
finishes.

The same change settles two more. A container that stopped part way is
recognised by its name, so it needs no metadata, no job row, and — the part
that mattered — no read-only open: `journal_mode = DELETE` is only set when an
import *finishes*, so a stopped one is still in write-ahead mode and fails to
open once its sidecar files are gone, which filed it under "Not readable" with
a raw SQLite error. Stopping before the first batch commits left a container
with neither metadata nor a job row, which was the same story again.

**Closing the sheet abandoned a running import.** The model was sheet-local
state, so Done discarded it and its Stop button while the detached importer
carried on writing — and reopening the sheet offered a fresh model that would
happily start the same import again, two writers resuming from one checkpoint
into one file. It is owned above the sheet now.

**Two quick taps on the permission switches overwrote each other.** Each saved
the whole row from the values it had read before the change, so the second
write restored what the first had changed and one switch silently flicked
back. One column is written now; the full row only when there is no row yet,
where the other columns need the defaults the catalogue was already showing —
including a sort order that came from the reader's languages rather than from
zero.

**A failed import left the list out of date**, because only the success and
stop paths asked for a reload; a run that threw can still have left a
part-written container, and a list that does not mention it is one the reader
cannot act on.

**A half-finished removal never reached the open reader.** Deleting the file
and forgetting the preference are two fallible steps, and the reload happened
only if both worked — so a reader whose preference failed to clear kept
reading from a container that was no longer there. The reload happens whatever
the outcome.

**A volume that is present but unreadable failed the whole discovery.** Under
a sandbox the reader's choice of one file need not grant access to its
siblings; importing two volumes of three with a diagnostic beats importing
none with an error. Scope is also requested on the containing directory, which
is granted in some cases and harmless when it is not.

And the duplicate reload: the status view still carried a `.task(id: phase)`
calling back on every terminal phase — and, since progress *is* the phase, on
every progress report. It was meant to go when the callback moved into the
model.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@widgetii

Copy link
Copy Markdown
Member Author

Two things in the second commit that its message does not mention, since they are documentation rather than fixes: M6 is ticked in the README roadmap, and the CHANGELOG gains the entries for everything in this milestone — importing your own dictionary, definitions with their structure intact, several dictionaries grouped by reading, and the three separate permissions.

Flagging it here rather than force-pushing an amended message, which would only restart a green CI run.

@widgetii
widgetii merged commit f95cda8 into main Oct 18, 2026
8 checks passed
@widgetii
widgetii deleted the feat/m6-import-ui branch October 18, 2026 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant