Repository navigation
feat: import a dictionary from inside the app - #32
Conversation
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>
PR Summary by QodoImport and manage dictionaries from inside the app
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
Code Review by Qodo
1. Multi-file imports fail in the sandbox
|
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>
|
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. |
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
shouldContinuealready 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
metaand 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
Listhas 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
And the detail panel, with both dictionaries grouped under one reading and each labelled:
5 new tests for the import model. Full suite green, lint clean, no warnings under
-warnings-as-errors.🤖 Generated with Claude Code