Repository navigation
Run CI on virtual Macs: skip the tests that need real OCR, and fall back to the CPU for OCR - #1
Merged
Merged
Conversation
On GitHub's Xcode 27 runner every test that reads text with Vision failed with empty text and no hint why. The assertions on OCR output now carry the extraction warnings, so a Vision error is told apart from Vision finding nothing. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
On GitHub's runners, which are virtual Macs, every Vision text recognition threw TextRecognition.CRImageReaderError 9: the accelerated path (Neural Engine or GPU) is not usable there, and every scan and photo lost its text. The same happens on Macs whose Neural Engine model fails to compile, and then keeps happening until the process restarts. OCRService now asks Vision through a TextRecognizing seam. When the default device fails, it reads the page again with every request stage on the CPU, and uses the CPU for later pages too; cancellation is never retried. The OCR trace records the device for each page. Tests reproduce the failure with a scripted recognizer (four fail without the fallback) and check that real Vision reads text on the CPU. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
hlistan
changed the title
The CPU fallback did not help on GitHub's runners: Vision fails there on the CPU too (an unknown error), so text recognition does not run on virtual Macs at all. The five tests that need real OCR now say so and are skipped, with that reason, where a direct Vision probe reads nothing; they run on every physical Mac, as the push protocol's local gates do. The probe calls Vision itself, so a defect in the app's OCR fails those tests rather than skipping them. The fallback stays for what it was measured to fix elsewhere: physical Macs whose Neural Engine model fails to compile. Comments and docs no longer claim it helps on virtual Macs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
swift test --quiet hid skipped tests along with passing ones, so a CI log could not show which tests a machine skipped. verify.sh now prints the skips and the totals when the tests pass, and the whole log when they fail. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
hlistan
changed the title
5 tasks done
hlistan
added a commit
that referenced
this pull request
Oct 7, 2026
…#17) ## What and why Fixes every finding of the QA run of 5 October 2026 (re-checked in a second run the same day), the review finding left standing at the merge of #16, and every finding of this change's fifty-three reviews; and hardens the guidelines so that no finding, of any rating, from a review, a QA run or a check, is ever left standing. One change, not split: the user asked for every QA finding fixed at once, and the guideline hardening is what the findings taught. Each fix below is independent and has its own regression test. | Finding | Fix | Kept by | |---|---|---| | ONB-4 Return went back a step in onboarding | each step's buttons are its own (`.id(step)`, `OnboardingView`) | QA protocol › C1 (no view test reaches AppKit's default button); re-checked in the second run | | RA-1 With Ollama away every waiting file was read for its text | once an item finds Ollama away, no other that needs the model is taken until it is tried again (`ollamaRetryAt`), in every queue; a file that comes is still looked at, so a copy goes to its original, and then waits unread (`JobStore.beforeTheModel`); Incoming, `run`, the menu bar and `ingest --json` say until when | `whileOllamaIsAway…` ×5, `aFileThatFailsWhileOllamaIsAwayLeavesTheWaitAsItIs`, `ingestNamesAFileThatFailedWithWhy`, `ingestNamesAFileThatWaitsWithWhyAsNoFailure`, `noWaitForOllamaIsSaid…`, `ingestWhileOllamaIsAway…`, `aStopped…` ×2, `aFileHeldWhileOllamaIsAway…`, `RuntimeActivityTests` | | SET-4 Settings changed by the command line were not heard by the app | `ChangeSignal` for the settings file, `SettingsStore.changes()`, another's change read under the settings lock | `aChangeAnotherProcessSavesIsHeardOfAtOnce`, `aChangeAnotherProcessPutsBackIsNeverInForceHere`, `aChangeWhoseRecordFailsOnceItIsSavedIsPutBack` | | READ-6 Fast joined a sender to a party, wrote references in the document's words and titles in capitals | a PDF's columns kept apart by a tab (`PDFPageText`, `extraction.pdf.columnGap`); what the document itself tells written at once in place of what is written otherwise, with a note (`AnswerValidator.toldByTheDocument`, `DocumentLayout`); what it does not tell sent back once | `columnsApart`, `LabelFormTests+Written` (eleven tests, about a hundred cases in a dozen languages and scripts); Fast measured before and after (below) | | CNV-8 No total per currency | the conversation prompt asks for a total of each currency apart (prompt 4) | `thePromptAsksForATotalOfEachCurrencyApart…` | | Review #4 of #16: Leave for Later in the instant of a rename was overwritten | the filing records the file where it is and keeps the rest (`FilingKept.movedOnly`), and no notification announces it | `aDocumentLeftForLaterAsItIsRenamed…` | | PL-5 / PL-6 (QA driver and protocol) | `qa-drive` looks again for its own window and names whose is there; the protocol moves `home/Indexes` aside | QA protocol | ## Acceptance criteria and proof - Every finding above has its regression test, shown to fail without its fix, or the protocol step that keeps it: `swift test` (1,102 tests) and `scripts/verify.sh --app` pass: static checks and gates, 1,102 tests, the fixture corpus, the release build, the app and the command. - ONB-4, RA-1 in the app and SET-4 were re-checked in the built app in a scratch home by the second QA run of the day. - What the document tells is never written otherwise than the document writes it: fifty-three reviews, the last forty-seven finding no character written into a title that the document does not write there; every condition of the title, reference and party rules is shown to fail a test when deleted (73 of 73 mutations). - Reading changes measured, Standard and Fast, version 11 then version 12, on the same Ollama server the same day (`arrumatorcli eval Tests/Fixtures --passes 2 --profile standard|fast --report`), in docs/evaluation.md: | profile | prompt | pass | type | sender | date | title | language | expected labels | parties found | |---|---|---|---|---|---|---|---|---|---| | Standard | 11 | 1 | 96% | 89% | 98% | 97% | 100% | 94% | 86% | | Standard | 12 | 1 | 93% | 89% | 98% | 95% | 100% | 96% | 91% | | Standard | 11 | 2 | 93% | 88% | 96% | 93% | 98% | 88% (a document lost to a server restart) | 82% | | Standard | 12 | 2 | 95% | 89% | 98% | 95% | 100% | 94% | 91% | | Fast | 11 | 1 | 88% | 86% | 86% | 93% | 97% | 66% | 86% | | Fast | 12 | 1 | 84% | 88% | 86% | 95% | 98% | 69% | 95% | | Fast | 11 | 2 | 88% | 84% | 88% | 93% | 97% | 68% | 86% | | Fast | 12 | 2 | 88% | 84% | 88% | 93% | 97% | 69% | 95% | Every fall is named in docs/evaluation.md with its documents: Standard's type (a DMV renewal read as a `license`, a social security proof's type changing between passes as it did in version 11), Standard's title (a JetBrains licence certificate titled after its subscription), Fast's type in pass 1 (a French tax notice, a Russian land register extract) and Fast's references in pass 2 (an invoice number in the document's Polish words, sent back once and left out when asked again). Fast's titles in capitals fell from 14 of 124 readings to 10, those of five documents that write their heading only in capitals. The eval ran on the build before the last two review rounds; their changes give none of the 14,718 capitals titles drawn from the fixtures another decision, and change the layout of one fixture, whose titles were all in sentence case. ## Test coverage Each fix began with a test that failed for the stated reason, shown by disabling the fix in place, then restoring and rebuilding. The title, reference and party rules were also mutated condition by condition in a copy of the package (each value of a set, each operand, each guard): all 73 mutations fail a test. Hostile input stays linear: a document of a million characters with no space and a title of 100,000 words are checked within a time limit, and the quadratic forms of the address scan and of the abbreviation check were shown not to finish. The Ollama wait is tested in every queue, across a stop and a restart, and on the time stored in the index. ## What changes for an installed app - **PDF text** keeps a page's columns and a label beside its value apart by a tab (PDF extractor version 4). Documents already read keep their text; a new document, or one read again, is read so. New `pipeline.json` key `extraction.pdf.columnGap` (2). - **Reading:** prompt version 12; a party, a title or a reference the document itself tells is written as it tells, with a note in the trace. Conversation prompt version 4. - **Command line:** `ingest` shows a file that waits for Ollama as the document it became, waiting, fails a file it failed, a document or not, and lists one it did not fail as queued, with what its job records, why and until when; `run` prints, line by line, when a file waiting for Ollama is tried again; `eval` waits for Ollama instead of scoring a document unread. - **App:** Incoming and the menu bar say until when work waits for Ollama; a filing left for later in the instant of its rename is announced by no notification. - No setting renamed, no learned state lost. ## Review Fifty-three fresh-context reviews read this change: the first read the whole change, and each later one read the fixes of the round before it. Every finding of every rating is fixed. Each fix has a regression test that was shown to fail with the fix disabled (red-checked), unless the fix only changes documentation or removes code. Tests are in `Tests/ArrumatorClassifyTests/LabelFormTests+Written.swift` unless another file is named, and a quoted title names the case that keeps the fix. Where a later round replaced a fix, the test named is the one that keeps the behaviour today. Severity follows docs/review/code-review.md: 4 Blocker, 3 Major, 2 Minor, 1 Nit. ### Review 1 — the whole change (the QA fixes and the hardened guidelines) | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | A reference lost the non-Latin letters of its number ("Plate ΙΚΤ 1234" became "1234"). | `AnswerValidator.described` stops at a word with a digit, a word in capitals or a word of another script. Words are dropped only where the document prints them as a field's name (`DocumentLayout.writesAsLabel`). | `aReferenceDescribedInEnglishStandsWhateverItsNumberIsWritten` (the ΙΚΤ, СА, АБ and 品川 cases); `aFieldsNameCopiedFromADocumentInAnotherLanguageIsLeftOutWhereItPrintsItBesideTheNumber` ("Série A 123") | | 2 | 3 Major | English reference words that a French or Romanian document shares ("Client 4711") were flagged and stripped. | A description whose words are all English (`EnglishWords`, NLEmbedding's vocabulary) stands. Otherwise the document's language must be preferred over English (`LanguageDetector.prefers`). | `aReferenceDescribedInEnglishStandsWhateverItsNumberIsWritten` (the fr and ro cases) | | 3 | 3 Major | The party "alone" repair cut names into fragments ("S.A.", "Depósitos", "- Maria Exemplo"). | `alone` is replaced by `DocumentLayout.joined`. A party is a join only when it equals two neighbouring tab-separated cells, one of them a sender's, and the other cell is written as printed. | `aPartyTheDocumentDoesNotPrintBesideItsSenderIsNoJoin` ("EDP Comercial S.A.", "Caixa Geral de Depósitos"); `aPartyJoinedToTheSendersNamePrintedBesideItIsWrittenAsPrinted` ("EDP - Maria Exemplo") | | 4 | 3 Major | `asSentence` misspelt titles: the Turkish dotted I, Greek accents and final sigma, and the brands NOS and IMI written in small letters. | Rewritten around the document's own spelling (`DocumentLayout`). Through review 7 this became the exact-writing rule. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("İHTARNAME KİRA SÖZLEŞMESİ", "ΣΥΜΒΟΛΑΙΟ ΜΙΣΘΩΣΗΣ"); `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("FATURA NOS TV") | | 5 | 2 Minor | A title of acronyms that the document writes among small letters ("IMI AT") was sent back. | The title goes back only when the document does not already write it that way: at first `asSentence(title) != title`, now `DocumentLayout.writesInCapitals`. | `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` (the "IMI AT" expectation) | | 6 | 2 Minor | `joinedWords = 2` was a tunable written as a literal. | Removed with the `alone` repair (#3). The join rule that replaced it has no tunable. | — (code removed) | | 7 | 3 Major | `arrumatorcli ingest --json` silently dropped the files held back while Ollama was away, and had no test. | Files not yet begun are named on standard error as queued, with exit code 0; docs/cli.md updated. | `ingestWhileOllamaIsAwayShowsEveryFileNotBegunAsQueued` (CommandLineTests+Stopping) | | 8 | 3 Major | The question queue's "waiting for Ollama until…" status was duplicated and had no test. | One rule, `ModelQueue.ollamaWait`, now serves both task queues. | `whileOllamaIsAwayNoOtherQuestionIsAnsweredUntilTheOneThatFoundItIsTriedAgain` (ConversationTests) | | 9 | 2 Minor | AGENTS.md said the ingest worker tells until when held-back files wait. It did not. | `IngestStatus.retryAt` and `JobProgress.waitingForOllama(until:)`, shown on the Incoming page; docs/using-arrumator.md updated. | `whileOllamaIsAwayNoOtherFileIsTakenUntilTheOneThatFoundItIsTriedAgain` (IngestQueueTests+Waiting) | | 10 | 2 Minor | The settings store could publish a change of its own that its failed record then put back. | A reread waits until a change under way is over: first through a `changing` guard, and since review 2 through the cross-process `SettingsLock`. | `aChangeWhoseRecordFailsOnceItIsSavedIsPutBack` (SettingsStoreTests) | | 11 | 2 Minor | A how-it-works sentence was false after the `movedOnly` fix, and a document left for later could be announced as filed. | The sentence is rewritten. A set-aside filing records `setAside`, and `EventRecord.announcesFiling` keeps it out of notifications. | `aDocumentLeftForLaterAsItIsRenamedIsRecordedWhereItIsAndKeepsTheRestAsTheUserLeftIt` (ReadingAgainTests+User) | | 12 | 1 Nit | A doc comment sat on the wrong declaration in `DocumentFiler.swift`. | Moved to `summary`. | — (documentation) | | 13 | 2 Minor | Pre-existing: "Waiting for Ollama" stayed on after nothing was waiting. | Cleared in `IngestCoordinator.refreshQueueCount` when no job is active. | `noWaitForOllamaIsSaidOnceNothingIsLeftThatWaits` (IngestQueueTests+Waiting) | | 14 | 2 Minor | The new guideline texts disagreed: a test or a protocol step, "approve even if not perfect", and "a change of its own". | AGENTS.md §2.4 and §7, docs/qa/protocol.md, docs/review/code-review.md and the PR template now state one rule. | — (documentation) | *Review 2 found 1, 2, 5–8 and 11–14 right. It found 3, 4, 9 and 10 only partly right, and its F1–F4 and F6 complete them.* ### Review 2 — the fixes of review 1 | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | F1 | 3 Major | A title in capitals took a word with different accents ("E" became "é", "À" became "a"). | Accents are compared where the title bears them, and nothing is written where the document spells the word more than one way. The exact-writing rule now does this. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("FATURA DE ÁGUA E SANEAMENTO", "TAXE FONCIÈRE À PAYER") | | F2 | 2 Minor | A heading word or form value printed in capitals beside small letters was kept in capitals ("FATURA de eletricidade"). | A word that the document writes both in capitals and otherwise is no longer an abbreviation. Now a word in capitals counts only beside a word of a sentence. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("FATURA DE ELETRICIDADE" beside the cell "FATURA n.º") | | F3 | 2 Minor | The party join still cut a name when a cell was a surname that the sender's name begins with ("Ana"). | A sender's cell needs the sender's whole name, or more than one word of it, and a party is never cut down to one word. | `aPartyTheDocumentDoesNotPrintBesideItsSenderIsNoJoin` ("Ana Maria Santos", "EDP Comercial Maria") | | F4 | 2 Minor | Pre-existing, and review 1 fixed it only for a store's own change: a reread could land inside another process's change that is then put back. | `readOthersChange` reads under the cross-process `SettingsLock`; AGENTS.md §4.6 updated. | `aChangeAnotherProcessPutsBackIsNeverInForceHere` (SettingsStoreTests) | | F5 | 2 Minor | After Ollama was away once, `arrumatorcli eval` scored a whole stretch of fixtures as "missing". | `IngestCoordinator.drain(waitingOutOllamaFor:)`, which the eval uses, waits out the hold; docs/evaluation.md and docs/cli.md updated. | `aFileHeldWhileOllamaIsAwayIsReadOnceItIsBackWhenItsDrainWaitsForIt` (IngestQueueTests+Waiting) | | F6 | 2 Minor | `arrumatorcli run` said "Waiting for Ollama…" without saying when Ollama is tried again. | `run` and the menu bar (`RuntimeActivity.Now.waitingForOllama(until:)`) now give the time. | `ollamaNotReadyHoldsFilingUpButWhatIsHappeningNowWaitsForItOnlyWhenSomethingDoes` (RuntimeActivityTests); `run`'s line is kept by `runSaysWhenAFileWaitingForOllamaIsTriedAgain` (review 3, #10) | | F7 | 2 Minor | A stopped queue still said it was waiting for Ollama. | `ModelQueue.stopWorker` forgets `ollamaRetryAt`. The ingest worker's `stop()` had the same fault and now clears its wait too. | `aStoppedQueueWaitsForNothingThoughItsTasksWaitedForOllama` (SearchTaskQueueStatusTests); `aStoppedWorkerWaitsForNothingThoughItsFileWaitedForOllama` (IngestQueueTests+Waiting) | *Review 3 found F3, F4, F5 and F7 right. It found F6 right while files wait (its #6 and #10 complete it) and F1/F2 only partly right (its #1–#4).* ### Review 3 — the fixes of review 2, and writing at once what the document tells | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | A title with no accents took the document's unaccented spelling even where the document also writes the word with accents ("Taxe foncière a payer"). | Fixed word by word, then by the whole-phrase rule (review 4): the title is the document's own phrase, and nothing is written where the phrase is spelled two ways. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("TAXE FONCIERE A PAYER"); `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("words spelled two ways") | | 2 | 3 Major | The layout included the vision model's description, so a card's title took the model's case and accents. | The layout is read only from the document's text and the e-mail's sender and subject (`ReadingGrounds`). | `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` (the vision-description case) | | 3 | 2 Minor | A heading word in capitals was taken for an abbreviation when only a number, punctuation or a one-letter word stood beside it ("FATURA de eletricidade", "TERMOS e CONDIÇÕES"). | Abbreviations were limited by length and by neighbours. Since review 7 the rule is replaced: a word in capitals must stand beside a word of a sentence. | `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("FATURA 2026/07", "TERMOS E CONDIÇÕES", "АКТ И СЧЁТ", "Assunto: RECLAMAÇÃO") | | 4 | 2 Minor | The title's first capital was recomputed from the language ("Ijzerhandel", "Ihtarname"). | At first the title's own capital was kept where the two disagreed. Since review 6 the app changes no letter's case and copies the document's own capital. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("IJZERHANDEL FACTUUR"); `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("İHTARNAME KİRA SÖZLEŞMESİ" read as English) | | 5 | 2 Minor | `writesAsLabel` checked less than its comment, the docs and the note claimed ("Contrato 12345" was cut to "12345"). | The field's name must end a cell, and the next cell, or the next line's first cell, must begin with the number. | `aFieldsNameCopiedFromADocumentInAnotherLanguageIsLeftOutWhereItPrintsItBesideTheNumber` ("Contrato 12345") | | 6 | 2 Minor | While the file that found Ollama away was tried again, the menu bar gave a retry time already past. | The wait is forgotten once its time has come, and no time is given while an item is in hand. A file being filed is shown first. | `whileAFileThatFoundOllamaAwayIsTriedAgainNoTimeIsSaid` (IngestQueueTests+Waiting); `whileARequestThatFoundOllamaAwayIsTriedAgainNoTimeIsSaid` (SearchTaskQueueStatusTests); `ollamaNotReadyHoldsFilingUpButWhatIsHappeningNowWaitsForItOnlyWhenSomethingDoes` (RuntimeActivityTests) | | 7 | 2 Minor | `drain(waitingOutOllamaFor:)` could spin once the retry time had passed and nothing could be taken. | Fixed with #6: the gate opens once its time has come, so the drain ends. | `aDrainThatWaitsForOllamaEndsWhenNothingIsLeftToTakeOnceItsTimeHasCome` (IngestQueueTests+Waiting, time-limited) | | 8 | 2 Minor | The `changing` guard was redundant with the lock, and no test failed without it. | The guard is removed. The cross-process lock alone keeps a read out of any change, the store's own included. | `aChangeWhoseRecordFailsOnceItIsSavedIsPutBack`, `aChangeAnotherProcessPutsBackIsNeverInForceHere` (SettingsStoreTests; both fail without the lock) | | 9 | 2 Minor | docs/evaluation.md described the superseded first design and its numbers. | Rewritten with the final numbers of this change, Standard and Fast, each measured against version 11 the same day | — (documentation: docs/evaluation.md, version 12 and Fast) | | 10 | 2 Minor | The new retry-time line of `arrumatorcli run` had no test. | `run` writes each line at once (`Run.say`), so a test can read it. | `runSaysWhenAFileWaitingForOllamaIsTriedAgain` (CommandLineTests+Stopping) | | 11 | 1 Nit | A leftover `sentBack: told("parties")` in a test suggested a send-back that cannot happen. | Removed. | — (test cleanup) | *Review 4 found 1–8, 10 and 11 right; #9 was not reviewed. Its F2–F5 complete 1 and 4.* ### Review 4 — the fixes of review 3 | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | F1 | 4 Blocker | The small letters inside an e-mail or web address counted as the document's spelling of a name ("Fatura vodafone julho"), and they also sent "FATURA DA EDP" back. | The letters of addresses, users' names and paths are blanked before words are read (`withoutAddresses`, a linear one-pass test of form). | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("FATURA VODAFONE JULHO"); `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` (the web and e-mail address cases) | | F2 | 3 Major | Accents were compared as a set, not by where they stand ("depositó" for "depósito"). | Whole-phrase rule: the title is the document's own run of words, each accent where it stands. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("COMPROBANTE DEL DEPÓSITO", "ENVÍO DOCUMENTOS") | | F3 | 2 Minor | A word printed only in capitals counted as an unaccented spelling, so "TAXE FONCIERE" was sent back. | Whole-phrase rule: only runs that hold small letters count. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("TAXE FONCIERE") | | F4 | 2 Minor | The dot above was set apart on every letter ("Zyciorys"). | It is set apart only after I or i, for the Turkish İ. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("ZYCIORYS ZAWODOWY") | | F5 | 3 Major | Two behaviours had no test: a first capital whose accent the capitals leave out, and a field's name on the line above its number. | Cases added. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("ETE INDIEN"); `aFieldsNameCopiedFromADocumentInAnotherLanguageIsLeftOutWhereItPrintsItBesideTheNumber` ("お客さま番号" above its number) | | F6 | 2 Minor | Nothing tested that `labels.abbreviationLetters < 2` is refused. | The setting was removed with the word-by-word rule. | — (code removed) | | F7 | 2 Minor | The menu bar said things that were false: a time while one wait had none, and "waiting" while an answer streamed. | No time is given if any wait has none, and work in hand is shown before waits. | `aQuestionTriedAgainWaitsForOllamaUntilTheModelBegins` (RuntimeActivityTests) | | F8 | 1 Nit | Leftovers: a status publish with no effect, a postponement time taken from a second clock reading, and three false doc comments. | The dead publish is removed and the comments fixed; `WorkOutcome.away` carries the one time `attempt` sets `ollamaRetryAt` to, and both queues postpone the item to it, so no second reading exists | `SearchTaskQueueStatusTests.whileOllamaCannotBeReached…` asserts the stored due time is the time the status says (red-checked against another time; two readings with no suspension between cannot be told apart by a deterministic test, so the fix rules them out by construction) | | F9 | 1 Nit | Docs and comments were wrong: "AIMA, I.P." was cited as an abbreviation, and the docs said "a line" where the code reads cells. | Corrected. | — (documentation) | | F10 | 1 Nit | A settings test threw away its own wait (`_ = await Patience.until`). | The tests now `#require` the wait. | `aChangeWhoseRecordFailsOnceItIsSavedIsPutBack`, `aChangeAnotherProcessPutsBackIsNeverInForceHere` (SettingsStoreTests) | *Review 5 found the validator's title check, `now`, the queues and the settings tests right. It found the layout rules as described, apart from its #1–#4, #6, #7 and #9.* ### Review 5 — the whole-phrase title rule | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | The first word lost the capitals inside it ("Iphone 15 Pro", "Ebay recibo"). | Since review 6 the app changes no letter's case: a title is written only from a run of words that the document begins with a capital. | `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("IPHONE 15 PRO") | | 2 | 3 Major | Falling back to the title's first letter wrote a letter the document does not write ("Ihtarname" in Turkish, or mixed widths). | Same fix as #1: nothing is written where the document does not write that capital. | `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("IHTARNAME KIRA SÖZLEŞMESI", "ＦＡＴＵＲＡ ＤＥ ＬＵＺ") | | 3 | 3 Major | Runs were compared without case and the first one won ("Caixa geral de depósitos"). Users' names also counted as the document's writing. | Runs are compared exactly, and a second spelling means nothing is written. A token holding `_` or `\` is blanked. | `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("CAIXA GERAL DE DEPÓSITOS", "Skype: Maria_Exemplo", the Windows path) | | 4 | 3 Major | A heading in capitals passed untold: it was neither sent back nor noted ("TERMOS E CONDIÇÕES", "CONSUMO EM kWh"). | A word in capitals counts only beside a word with small letters (since review 7, beside a word of a sentence). | `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("TERMOS E CONDIÇÕES", "NOTA DE CRÉDITO", "CONSUMO EM KWH") | | 5 | 2 Minor | A title that mixes an abbreviation with a script without case ("NTT 請求書") was sent back. | `inCapitals` counts only words that hold a cased letter. | `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` (the `inCapitals` expectations) | | 6 | 2 Minor | On hostile input, `asSentence` grew with title length times the number of matches (24 s and 747 MB). | Words are folded once and runs are compared without copies. A title longer than `naming.maxChars` is never looked for (`titleMaxChars`). | `aLargeDocumentIsLaidOutInPassesLinearInItsSize` (the 200-word title) | | 7 | 3 Major | No test pinned reading by cells rather than lines, the dot rule for addresses, or the first-pass send-back. | Cases added, and every untold case now asserts that it goes back on the first pass (`goesBackOnce`). | `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("words a tab parts"); `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("EDP COMERCIAL S.A. LISBOA") | | 8 | 2 Minor | docs/evaluation.md described a validator the code no longer has, with numbers from before this round. | Rewritten with the final numbers of this change, Standard and Fast, each measured against version 11 the same day | — (documentation: docs/evaluation.md, version 12 and Fast) | | 9 | 1 Nit | Greek titles with accents that end in Σ were never repaired (the final sigma). | The final sigma was folded with σ; titles are now compared through the document's own capitals. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("ΌΡΟΙ ΧΡΉΣΗΣ") | | 10 | 1 Nit | `RuntimeActivity.now` had an unreachable line. | Removed. | — (code removed) | *Review 6 found `asSentence`, the title bound, `now` and the callers right.* ### Review 6 — exact spelling and sentence context | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | A ligature at the start of the first word made "Fnal notice". | The app no longer changes the case of any letter. It copies, letter for letter, a run of words that the document begins with a capital. | `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("FINAL NOTICE" over "final"); `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("FINAL NOTICE") | | 2 | 2 Minor | With the language unknown, Dutch "ij" got the root capital ("Ijzerhandel"). | Same rule as #1: the capital is the document's own. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("IJZERHANDEL FACTUUR") | | 3 | 2 Minor | Root case rules dropped correct repairs of Turkish and Greek titles. | A run matches the title when its words put into capitals, by the document's language or by the root rules, are the title's words (`spells`). | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("DOĞALGAZ FATURASI", "ΤΙΜΟΛΟΓΙΟ ΜΑΪΟΥ", "İHTARNAME KİRA SÖZLEŞMESİ") | | 4 | 3 Major | Three behaviours had no test: the first-letter exemption, the `/` rule and the `\` rule. | The exemption went away with the case change. Cases were added for a sentence-case heading the document writes twice and for both kinds of path. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("FATURA DE ELETRICIDADE"); `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("/srv/Maria/Faturas", the Windows path) | | 5 | 2 Minor | The long run in the hostile-input test never exercised the scan in `isAddress`. | Added a long run with no marker ("ab-1" × 250,000). | `aLargeDocumentIsLaidOutInPassesLinearInItsSize` (red-checked: with an `isAddress` that copies the rest of the run at each character it did not finish in 4 minutes; 13 s with the fix) | | 6 | 2 Minor | `writesInCapitals` cost title words × document words. | The document's abbreviations are read once (`abbreviations`). | `aLargeDocumentIsLaidOutInPassesLinearInItsSize`: a 100,000-word title in capitals over the large document (red-checked: with each word looked for in all of it again it did not finish in 10 minutes) | | 7 | 1 Nit | Docs and comments were no longer true (the how-it-works title rule, `Cell`, `inSentence`, `naming.maxChars`). | Corrected in docs/how-it-works.md, docs/using-arrumator.md and `DocumentLayout`. | — (documentation) | *Review 7 found no false match, no character written that the document does not write at that place, and a linear cost.* ### Review 7 — the exact-writing title rule | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | F1 | 3 Major | Nothing tested that a title with accents must match them exactly; the two cases meant to test it did nothing. | The decoy is given a capital start, and a single-run case that must go back is added. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("ENVÍO DOCUMENTOS"); `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("ENVÍO DOCUMENTOS", "the words with other accents") | | F2 | 3 Major | The Turkish-dot exceptions and the "holds a small letter" clause had no test. | Cases added, and `marks` and `unmarked` now share one predicate (`isMark`). | `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("IHTARNAME KIRA"); `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("DOGALGAZ FATURASI İSTANBUL", "FATURA EDP") | | F3 | 2 Minor | A heading word beside "n.º", "nº" or a unit ("kWh") counted as an abbreviation, so the title stayed in capitals. | A word of a sentence (`Word.ofSentence`) is not joined to another word by a dot and has no capital after its first letter. | `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("FATURA EDP" beside "FATURA n.º"); `aHeadingBesideANumberSignAUnitOrANumbersNameIsNotWrittenAnew` ("RECIBO EDP", "CONSUMO EDP") | | F4 | 1 Nit | A stale comment: its Dutch "IJ" example is about title case, not capitals. | Replaced with examples of capitals: the Turkish "İ" and the Greek tonos. | — (documentation) | *The author also built the `Locale` once and looked the title up once, two costs the review noted. Review 8 found these changes equivalent, found no false match, and found that each new case fails without its part of the rule.* ### Review 8 — words of a sentence | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 2 Minor | Pre-existing; review 7 fixed it only for Portuguese spellings: a heading beside "No.", "Nr.", "n°", "nr", "Αρ." or "No:" still passed as an abbreviation. | A word followed by a number, with only spaces or ".", ":" or "°" between them, names that number and is no word of a sentence (`naming`). | `aHeadingBesideANumberSignAUnitOrANumbersNameIsNotWrittenAnew` ("INVOICE No. 4711", "RECHNUNG Nr. 4711", "INVOICE NO. 4711") | | 2 | 3 Major | The `inSentence` half of review 7's fix, and `small &&` in `ofSentence`, had no test. | Cases added, and the repairs now declined (`S.p.A.`, `eBay`) are pinned. | `aHeadingBesideANumberSignAUnitOrANumbersNameIsNotWrittenAnew` ("CONSUMO KWH", "RECIBO Nº 12", "FATURA EDP"); `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("FATTURA ENEL ENERGIA S.P.A.", "COMPRA EBAY UK") | | 3 | 1 Nit | The comment on `AnswerValidator.written` no longer matched the rule. | It now says "beside a word of a sentence (`DocumentLayout.writesInCapitals`)". | — (documentation) | *Review 9 found no wrong character, no guess wrongly kept and a linear cost, and found that the INVOICE and RECHNUNG cases fail without the change.* ### Review 9 — a number's name | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | Four new cases passed without the rule they name, because each sender stood alone on its line. | Each sender now stands beside a word of a sentence ("EDF Commerce", "PGE Obrót", "ΔΕΗ Ανανεώσιμες", "TTNET Anonim"). | `aHeadingBesideANumberSignAUnitOrANumbersNameIsNotWrittenAnew` (red-checked without the colon, without the degree sign and without the spaces-only form) | | 2 | 2 Minor | Pre-existing: a number's name before a number that begins with letters ("No. INV-2026-118") was still a word of a sentence, and "FV/2025/123" was blanked as a path. | What follows the signs is a number when it holds a digit (refined in reviews 10–12), and a slash between digits no longer marks a path. | `aHeadingBesideANumberSignAUnitOrANumbersNameIsNotWrittenAnew` ("INVOICE No. INV-2026-118", "INVOICE NO. INV-2026-118", "FAKTURA nr FV/2025/123") | | 3 | 1 Nit | "º" in `numberSigns` could never apply, because it is a letter. | Removed, along with "nº 12" and "ordinal sign" in the comments. | — (dead entry removed) | | 4 | 1 Nit | The docs called the rule "the name of a number", though it covers any word a number follows. | docs/how-it-works.md and the `Word` comment now say "a word a number follows, as a number's name or a month before its year". | — (documentation) | *Review 10 found that the three new cases fail without the change and that removing "º" changes nothing, but its #2 shows the narrower path rule was a regression.* ### Review 10 — number chunks and paths | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 2 Minor | Any chunk holding a digit made the word before it a number's name, which dropped repairs of "COVID-19", "AA-12-BB" and "Q3". | A chunk is a number when it begins with a digit or holds more digits than letters. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("CERTIFICADO DE VACINAÇÃO COVID-19", "SEGURO AUTOMÓVEL AA-12-BB", "RELATÓRIO Q3 2025") | | 2 | 3 Major | Review 9's narrower path rule let relative, home and drive paths through, so a folder name's letters could be written into a title. | A slash beside a letter marks a path again, and an address keeps its digits and loses only its letters. | `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("~/Maria-Faturas", "C:/Maria-Exemplo", "Arquivo/2026/Contrato De Arrendamento") | | 3 | 3 Major | No test failed without the "at a path's start" branch. | Case added. | `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("/Maria-Faturas") | *Review 11 found no character written that the document does not write and a linear cost. The COVID-19, AA-12-BB, Q3 and path cases fail on the previous code.* ### Review 11 — the number and path fixes | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | The "letter before the slash" half of the path rule had no test, and it blanked "julho/2026", which dropped correct repairs. | Dropped: only a slash followed by a letter marks a path. The kept-digits rule gets a case of its own. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` ("RECIBO DE VENCIMENTO JULHO 2026"); `aHeadingBesideANumberSignAUnitOrANumbersNameIsNotWrittenAnew` ("FAKTURA nr FV_2025_123") | | 2 | 2 Minor | After "No." or "Nr.", a reference with as many letters as digits ("ABC123") no longer made the word before it a number's name. | After a sign, any chunk with a digit is the number (narrowed to a dot or a degree sign in review 12). | `aHeadingBesideANumberSignAUnitOrANumbersNameIsNotWrittenAnew` ("INVOICE No. ABC123") | | 3 | 3 Major | The "begins with a digit" clause had no test. | Case added. | `aHeadingBesideANumberSignAUnitOrANumbersNameIsNotWrittenAnew` ("INVOICE nr 1A") | | 4 | 1 Nit | Two comments still said an address's words are left out, though its digits now stay. | They now say "the letters of". | — (documentation) | *Review 12 found no character written that the document does not write and a linear cost, and found that each new test fails without the code it names.* ### Review 12 — signs and slashes | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 2 Minor | After a colon, "any digit" made a word a number's name, which dropped repairs such as "Relatório: Q3 2025" and "Seguro automóvel: AA-12-BB". | "Any digit" applies only after a dot or a degree sign (`namesEnd`); after a colon the spaces rule applies. | `aTitleInCapitalsIsWrittenAsTheDocumentWritesItsWords` (the two colon cases); `aHeadingBesideANumberSignAUnitOrANumbersNameIsNotWrittenAnew` ("FACTURE n° AB12") | | 1a | — (found by the author) | Found while fixing #1, on the fixture corpus: in "Cad. No:1", the colon glued "No" to "1" into one chunk with more letters than digits, so "No" became a word of a sentence. | What follows the sign is counted from the run after it onward (`rest`). | `aHeadingBesideANumberSignAUnitOrANumbersNameIsNotWrittenAnew` ("FATURA No:1") | | 2 | 1 Nit | The claim "never from the letters of … a path" said too much: a path whose slashes all come before digits is read as words. | Narrowed in `DocumentLayout`'s comments and docs/how-it-works.md to a path recognised by a slash before a letter. | — (documentation) | | 2a | — (found by the author) | Applying the coverage rule this round taught (each value of a set a rule treats alike has its own test): "@" had no case of its own, as every e-mail case also held a dot with two letters, and "://" covered no case the slash before a letter did not. | "://" removed; a social handle pins "@". | `aTitleTheDocumentDoesNotTellTheSmallLettersOfIsNotWrittenAnew` ("@Fatura-Eletronica", red-checked) | ### Review 13 — the colon rule, counting after a sign, the address rule and the Ollama retry time The reviewer mutated a copy of the package (each condition deleted in turn) and reported every condition no test failed without; each now has a case, and the author re-ran every mutation of rounds 12 and 13 against the real tests : all 73 make a test fail. | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 2 Minor | No test failed without the digit condition after a dot or without the guard that only spaces and number signs stand between a word and its number. | Cases for both. | `aTitleOfWordsTheDocumentWritesInCapitalsBesideASentenceIsAsAsked` ("…em maio. IMI", "IMI pago (2025)") | | 2 | 2 Minor | Other conditions no test failed without: the e-mail's sender and subject being laid out, an ordinal sign being no small letter, the sender's cell coming second in a joined party and its one-word guard, a cell that starts the sender's name, a field's name at a cell's end, and the word after a dot. | A case for each. | `aTitleIsWrittenAsAnEmailsSubjectOrSenderWritesIt` (both keys); "FATURA Nº FT 2026/1"; `aPartyJoinedToTheSendersNamePrintedBesideItIsWrittenAsPrinted` and `aPartyTheDocumentDoesNotPrintBesideItsSenderIsNoJoin` ("Maria EDP Comercial"); `aFieldsNameCopied…` ("Tipo de documento Fatura n.º"); "FATTURA IVA" | | 3 | 1 Nit | Two guards no input reaches: the small "i" before a dot above in `isMark`, and grounding a joined party, which is the document's own cell. | Both deleted. | — (code removed; AGENTS.md §3 now says a guard no input reaches is deleted) | | 4 | 1 Nit | With a colon following the spaces rule, "INVOICE No: ABC123" is read as words, as "Relatório: Q3 2025" is. | Kept as the price of "Relatório: Q3 2025", "Seguro automóvel: AA-12-BB"; said in docs/how-it-works.md and pinned. | `aTitleOfWordsTheDocumentWritesInCapitalsBesideASentenceIsAsAsked` ("INVOICE No: ABC123") | | 5 | 1 Nit | Before this change: after a dot or degree sign, a number begun by its series' letters apart ("No. FT 2026/1") did not make the word before it a number's name. | A rule for it was made, then withdrawn after review 14 (#2) showed it takes a sentence's end for a number's name; "No. FT 2026/1" is read as words, as before this change, said in docs/how-it-works.md and pinned. | `aTitleOfWordsTheDocumentWritesInCapitalsBesideASentenceIsAsAsked` ("INVOICE No. FT 2026/1", "FATURA No FT 2026/1") | | 6 | 1 Nit | Comments and docs said more or less than the code: "what follows up to the next space", "any run with a digit", the address rule after "://" went, and a path told only by a slash. | Rewritten to the code's own terms: the five marks of an address listed, "http://10.0.0.1" and "Arquivo/2026" named as read. | — (documentation) | | 7 | 1 Nit | The new coverage rule covered sets of values, but most untested conditions were operands and guards. | AGENTS.md §3 and checklist T1: each condition of a rule, each value of a set, each operand of a compound condition and each case of a `switch` is tested on its own, shown by deleting it; a guard no input reaches is deleted. | — (guidelines) | ### Review 14 — the whole pull request, fresh context | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 2 Minor | While Ollama was away the ingest worker took no job at all, so an exact copy put into Incoming was not handed over to its original, nor a file gone recorded, until Ollama came back. | While Ollama is away the worker takes only a file that came and is not hashed yet (`JobStore.beforeTheModel`): a copy goes to its original and the Trash, a file gone is recorded, and any other waits before its text is read, in its place, with no History line; a document read or indexed again waits as it is. `ingest --json` lists such a file as the document it became, waiting. | `whileOllamaIsAwayAFileThatComesIsStillHandedOverToItsOriginalOrRecordedGone`, `whileOllamaIsAwayOnlyAFileThatCameAndIsNotHashedYetIsTaken`, `whileOllamaIsAwayNoOtherFileIsReadForItsTextUntilTheOneThatFoundItIsTriedAgain`, `ingestWhileOllamaIsAwayShowsEveryFileAsItWaitsAndOneNotBegunAsQueued` (all red-checked against the old gate; 11 mutations of the new rule, each failing a test, two unreachable conditions deleted) | | 2 | 2 Minor | The series-letter rule of review 13 #5 took a sentence's end for a number's name ("…em maio. IMI 2025", "IMI liquidado. A 15 de maio"), sending a right title of abbreviations back. | The rule is withdrawn: a correct title sent back costs a call and risks a weak model's answer, while "No. FT 2026/1" read as words only lets a capitals title stand as the model gave it. Both readings pinned and said in docs/how-it-works.md. | `aTitleOfWordsTheDocumentWritesInCapitalsBesideASentenceIsAsAsked` ("…maio. IMI 2025", "…liquidado. A 15 de maio", "INVOICE No. FT 2026/1"; the two sentence ends fail with the rule restored) | ### Review 15 — the fixes of review 14 | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | A file taken while Ollama is away that failed before the model, as one that cannot be read, cleared the wait, so the next job's text was read only to wait for Ollama too. | A failure of a job taken while Ollama is away says nothing of Ollama: the wait and the status stay (`handleFailure`, `takenWhileAway`). | `aFileThatFailsWhileOllamaIsAwayLeavesTheWaitAsItIs` (red-checked) | | 2 | 2 Minor | `Reached` recorded what `takenWhileAway` already did; deleting it failed no test. | `Reached` deleted: the trace of a job that waits before its text ends as waiting by its stage. | — (code removed) | | 3 | 2 Minor | Five comments still said no job is taken while Ollama is away. | Each says what is done now: no job is read for its text, and only a file that came and is not hashed yet is taken. | — (comments) | | 4 | 2 Minor | `ingest` called a file that failed before it became a document "queued, not read yet" and exited 0, dropping why. | Only a file with no failure is not begun; one that failed is named with why, and the command exits with 1, as docs/cli.md says. | `ingestNamesAFileThatCouldNotBeReadWithWhy` (red-checked) | ### Review 16 — the fixes of review 15 | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | The test of a file named "with why" passed without the why. | It asserts the job's own recorded reason on standard error. | `ingestNamesAFileThatFailedWithWhyAndOneThatWaitsAsQueued` (red-checked against the fallback reason) | | 2 | 2 Minor | A file waiting for the archive's folder, which spends no attempt but keeps its last error, was called a failure. | A file is not begun while it has spent no attempt (`attempt == 0`), as every failure that leaves a job queued spends one and a wait spends none. | the same test, its waiting case (red-checked against the last-error rule) | | 3 | 2 Minor | The two new tests, and two older ones, failed rather than skipped when run as the superuser, who opens a file whatever its permissions. | Each is enabled only for another user, saying why, as `WatchingTests` does; the record file's case that needs it is a test of its own. | `aFileThatFailsWhileOllamaIsAway…`, `ingestNamesAFileThatFailed…`, `aRecordFileNobodyMayReadIsNeverTakenForAbsent`, `anExportTheArchiverFailsToPack…` | | 4 | 1 Nit | Two comments said "a job that ended before the model" where the code tests a job taken while Ollama is away. | Worded by what is tested. | — (comments) | ### Review 17 — the fixes of review 16 | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 2 Minor | For a file waiting for the archive's folder the command said "queued, not read yet" without why, where Incoming shows the reason and when it is tried again; the command decided by a rule of its own. | A file that waits is named, in the list and on standard error, with why it waits (refined in review 18: as Incoming says it, from `IngestStatus.progress(of:)`). | `ingestNamesAFileThatWaitsWithWhyAsNoFailure` (red-checked against the last-error rule and against a message without the reason) | | 2 | 2 Minor | docs/cli.md still said only a file waiting for Ollama is no failure. | It says a file waiting for Ollama or the archive's folder is none, and is named with why. | — (documentation) | | 3 | 2 Minor | Fourteen more tests relied on permission bits the superuser ignores, unguarded, and the new CLI test guarded a half that needs none. | Two shared traits in `Tests/Support` (`.fileModesKeepOut`, `.folderModesKeepOut`) on all twenty such tests, the inline copies of the reason replaced; the CLI test split, its waiting half unguarded; AGENTS.md §3 names the traits. | the twenty tests, skipped for the superuser with the reason | ### Review 18 — the fixes of review 17 | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | What the command says of a file not begun was tested only up to "queued, not read yet", and its line in the list not at all. | Both tests assert the whole line on standard error and in the list, once with a reason and once without. | `ingestWhileOllamaIsAway…`, `ingestNamesAFileThatWaitsWithWhyAsNoFailure` (five mutations of the output, each failing a test) | | 2 | 2 Minor | A settings test was skipped for the superuser though its first half, History refusing the change, needs no permissions. | Split in two; only the half that closes the folder is guarded. | `aChangeWhoseRecordIsRefusedIsNotSaved`, `aChangeWhoseSettingsCannotBeSavedIsNotRecorded` | | 3 | 2 Minor | A file that failed once and waits now was called a failure, the wait's message its reason, as "spent an attempt" asked of the job's whole life. | The command asks what this run did: a file it spent an attempt on is a failure; one it spent none on is queued (`JobRecord.failedAnAttempt` removed). | `ingestNamesAFileThatWaitsWithWhyAsNoFailure` (a file that failed before, red-checked) | | 4 | 1 Nit | The command said why a file waits but not when it is tried again, which Incoming says. | It says it as Incoming does, from `IngestStatus.progress(of:)`: the reason and when it is tried again, or that it waits for Ollama until then. | the same test (red-checked against a line without the time) | ### Review 19 — the fixes of review 18 | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | The "until when" the command says was proven by no test: the assertions stopped at "tried again at", and two of its cases were reached by none. | The tests assert whole lines with the date, and each case of the wording has one. | `ingestNamesAFileThatWaitsWithWhyAsNoFailure`, `ingestWhileOllamaIsAway…`, `ingestLeavesAFileAnotherProcessFailsMeanwhileQueued` (seven mutations, each failing a test) | | 2 | 2 Minor | The command described a file another process holds by its own process's live state, as its own wait for Ollama, or a time already past. | It says what the job records, whichever process has it: why a stage last left it and when it is tried again while that is to come, else that the app or `run` files it. | the same tests (a held file whose time has come) | | 3 | 2 Minor | "This command spent an attempt" measured an attempt spent by any process while it ran, so the app's failure meanwhile failed the command. | The drain says which jobs it spent an attempt on (`IngestCoordinator.drain` returns them); the command fails only those. | `aDrainSaysWhichJobsItFailed` (Core, needs no permissions), `ingestLeavesAFileAnotherProcessFailsMeanwhileQueued` | | 4 | 2 Minor | The attempt read before the drain was a second database access with a fallback no input reached. | The read is gone, as the drain says what it failed. | — (code removed) | ### Review 20 — the fixes of review 19 | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | Before this change: a file the command failed once it had become a document did not fail the command. | A file is a failure when this command recorded a failure of it, with a document or not. | `ingestFailsAFileWhoseReadingFailedHereOnceItBecameADocument` (a loopback stand-in for Ollama answers the reading with HTTP 500; red-checked) | | 2 | 2 Minor | The tests formatted the expected time in their own time zone, the command in the system's. | The command's environment carries the test's time zone (`Home.environment`, shared by `run` and `launch`). | the waiting tests, passing under another `TZ` | | 3 | 2 Minor | What the drain returns was tested only on a fresh job filed and one failed; three wrong forms passed. | `handleFailure` says whether it recorded a failure (an attempt kept, or the job ended failed), and the drain returns those; every branch of that answer is tested. | `aDrainSaysWhichJobsItFailed` (filed after an earlier failure, read again as it changed, retried, set aside, set aside while the archive is not there, final, unreadable payload, waiting for a model, the archive or Ollama), `aJobCancelledAsItsFailureIsSavedIsNoFailureTheDrainRecorded`, and the claims tests (thirteen mutations, each failing a test) | | 4 | 2 Minor | The docs and comments said a file the app fails meanwhile is queued, but one the app ends failed fails the command; and the drain's set held unsaved attempts and missed failures that spend none. | Code and docs agree: the drain returns the failures it recorded; the command fails those and any file whose job ended failed, whoever ended it, and lists the rest queued. | the tests above; docs/cli.md | | 5 | 1 Nit | Two doc lines of `process` began alike. | Merged. | — | ### Review 21 — the fixes of review 20 | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | The drain kept every job that failed at any time in it, so a file that failed, was taken again in the same drain and filed failed the command. | The drain keeps each job's last outcome in it. | `aFileTakenAgainInTheSameDrainEndsAsItsLastAttempt` (filed, and waiting for Ollama, after a failure; red-checked) | | 2 | 3 Major | Four answers had no test: a lost claim as an unreadable payload, a final refusal or a last attempt was saved, and the look for a missing model. | A lost claim is one path: saving a failure throws when the job is no longer the worker's, and `handleFailure` ends it once, its trace cancelled; the look for a model has its test. | `aJobCancelledAsItsFailureIsSavedIsNoFailureTheDrainRecorded`, the claims tests, `aFileWaitingForItsModelFailsNothingWhenItLooksAgain` (sixteen mutations, each failing a test; the answer for a file read again from the start is replaced by its next attempt in the same drain, which it is due for at once, as review 22 showed, a test now observes it) | | 3 | 2 Minor | A job cancelled as its retry was saved left its trace running. | Ended as cancelled by the one path above. | the same test, its trace's outcome | | 4 | 1 Nit | Wording of the drain and of `handleFailure`; a `@discardableResult` nothing discards; docs/cli.md on waits. | Reworded; removed. | — | ### Review 22 — the fixes of review 21 | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | The answer for a file read again from the start can be observed: the drain does not take a job again while work its reading gave up on still runs. | Tested so. | `aFileReadAgainFromTheStartIsNoFailureThoughNotTakenAgainInTheDrain` (red-checked) | | 2 | 3 Major | No test lost the claim at most saves of a failure: the waits for Ollama, the archive and a model, a final refusal, an unreadable payload, a file read again, setting aside. | The separate check of the claim before them is gone: each path's first save is the check, so a job cancelled while it is read reaches every one; once a failure is kept, a claim lost after it as the file is set aside changes nothing of it. | `aJobCancelledWhileReadIsNoFailureHoweverItsReadingFails` (eight ways), `aFailureKeptStandsThoughTheJobIsCancelledAsItsFileIsSetAside` (thirteen mutations, each failing a test) | | 3 | 3 Major | Before this change: a document read again whose last attempt failed as the user left it for later was marked failed over the user's decision. | It is marked failed only while the job's claim holds, checked in a write just before, as a file is before it is moved. | `aDocumentLeftForLaterAsItsReadingAgainFailsStaysAsTheUserLeftIt` (red-checked) | | 4 | 2 Minor | docs/cli.md and two comments said "no failure" of a file that waits to be set aside, a failure kept. | Worded by how the file's last attempt ends: waiting with no failure kept is none. | — (documentation) | | 5 | 1 Nit | A lost claim still ended in two places. | One: the check before the saves is gone (`JobStore.holds` with it). | — | ### CI of the tenth commit — two command-line tests timed out | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | On a loaded runner two tests against the loopback stand-in for Ollama failed ("Ollama timed out: api/show"): the command waits 2 s for such an answer, as a real local server needs, and the test measured the runner's speed. | Tests run against the stand-in give the command patient timeouts through their home's settings (`LoopbackOllama.patient`, `Home.make(pipeline:)`). | the two tests and the HTTP 500 one: with every stand-in answer delayed 3 s they pass, and fail as on CI without the patient timeouts | ### Review 23 — the fixes of review 22 | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | The claim check before a document read again is marked failed was a write of its own, so leaving it for later could land between it and the change. | The check, the document marked failed and History's record are one write, as filing is; so is the record of a failure that left no document. | `aDocumentLeftForLaterAsItsReadingAgainFailsStaysAsTheUserLeftIt`, `aFileThatFailsBeforeItIsADocumentRecordsNothingOnceItsJobIsCancelled` (red-checked) | | 2 | 3 Major | The test of a job cancelled while read did not look at its document or History: marking it failed, or leaving its file in Incoming, before the first save passed. | It asserts the document is not marked failed and History records no failure, retry or error, in all eight ways. | `aJobCancelledWhileReadIsNoFailureHoweverItsReadingFails` (both moves red-checked) | | 3 | 2 Minor | The drain's comment left out a file read again from the start; a job taken over as it waits to be set aside left a trace saying it waits. | Named; the trace ends as cancelled, the failure still recorded. | `aFailureKeptStandsThoughTheJobIsCancelledAsItsFileIsSetAside` | | 4 | 2 Minor | Before this change: `arrumatorcli review hold` left a document still being read in for later, and its reading then filed it over that; the app offers no such choice. | Refused while the document is read in (`IngestError.beingReadIn`), as the app; docs/cli.md says so. | `aDocumentBeingReadInIsNotLeftForLaterUntilItIsRead` (red-checked) | ### CI of the eleventh commit — the loopback stand-in starved | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | A dry run against the loopback stand-in never ended: the stand-in answered on Dispatch's shared pool, which the tests waiting on their commands hold on a small runner, as the earlier time-out was the same. | The stand-in listens and answers on threads of its own, with plain sockets, as `ChildProcess` waits on its own; AGENTS.md §3 says so of every double that serves another process. | with Dispatch's pool filled with blocked work, the former stand-in never listened and the command failed; this one answers it | ### Review 24 — the twelfth commit | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | Two conditions of the refusal to leave a document for later while it is read in had no test: that it is this document's reading, and a file put into the archive. | Tested. | `aDocumentBeingReadInIsNotLeftForLaterUntilItIsRead` (another document held meanwhile; a file adopted; both red-checked) | | 2 | 3 Major | Before this change: nothing tested that a document read again whose last attempt fails is marked failed, saying why, and recorded in History. | Tested. | `aDocumentWhoseReadingAgainFailsItsLastAttemptIsMarkedFailedSayingWhy` (status, problem and event each red-checked) | | 3 | 3 Major | Before this change: a filed document read again for search after a rebuild whose reading failed was marked failed, over the user's leaving it for later. | A document the user set aside stays as the user left it (refined in review 25, which found a filed one left with no way back). | `aDocumentWhoseReadingForSearchFailsWaitsForTheUserUnlessSetAside` | | 4 | 2 Minor | The loopback stand-in set `SO_NOSIGPIPE` on each connection, which fails on one the client already reset, so its write could end the test process. | Set on the listening socket, which every connection inherits. | with 200 clients resetting before their answers, the former stand-in's process died of SIGPIPE three times in three, this one never | | 5 | 2 Minor | The stand-in's address could no longer be nil, and three tests still waited for it. | `let address: String`; the waits removed. | — | | 6 | 1 Nit | A check passed when the document was missing. | It requires the document, and expects it as it was. | `aJobCancelledWhileReadIsNoFailureHoweverItsReadingFails` | ### Review 25 — the thirteenth commit | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | Kept filed after its reading for search failed, a document had no way back: not in Needs You, no Read Again on its card, and History worded it as any failure. | A document whose reading for search fails is marked failed and waits in Needs You to be read again, as before this change; only a document the user set aside stays as the user left it. History says its reading for search failed. | `aDocumentWhoseReadingForSearchFailsWaitsForTheUserUnlessSetAside` (filed and held; three mutations, each failing a test) | | 2 | 3 Major | The other kinds of job that mark a document failed where it is had no test: a file put into the archive, a file in Incoming gone by its last attempt. | Tested. | `aFileReadInWhoseLastAttemptFailsWhereItIsIsMarkedFailed` (both) | | 3 | 2 Minor | docs/storage.md, docs/architecture.md and a comment said more than the code. | Rewritten to the rule above. | — (documentation) | | 4 | 1 Nit | The stand-in did not check that `SO_NOSIGPIPE` was set. | It throws when it is not, as when it cannot listen. | — | ### Review 26 — the fourteenth commit | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | A document undone after its reading for search was queued, whose reading then failed, was kept undone, untested. | Tested, with its reading's job following its file into Incoming. | `aDocumentWhoseReadingForSearchFailsWaitsForTheUserUnlessSetAside` (filed, held, undone; red-checked) | | 2 | 2 Minor | Before this change: a document undone in the instant between its filing and its job's end, followed by a stop, was filed again by that job when taken up. | Undo refuses a document whose reading in has not ended, as leaving it for later does (`IngestError.beingReadIn`); docs/cli.md says so. | `aDocumentWhoseReadingInHasNotEndedIsNotUndoneNorLeftForLater` (red-checked) | | 3 | 1 Nit | Comments said leaving a document for later cancels its job, which holds of a reading again alone. | Worded as the code does: a reading again is cancelled, a reading in refused, and a reading for search after a rebuild is what is left. | — | ### Review 27 — the fifteenth commit | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 2 Minor | While a document was still being read in, its card offered Undo Filing, and Leave for Later in Needs You, which Core then refused, naming the document by its id. | `ReviewActions.choices` leaves both out until its reading in ends, and the card looks again as the queue moves on; the refusal names the file. | `aDocumentWhoseReadingInHasNotEndedIsNotUndoneNorLeftForLater` (red-checked); QA protocol C3 | | 2 | 2 Minor | Before this change: an exact copy dropped in Incoming was handed over to an original the user undid meanwhile, which was then filed again. | A copy is handed over only to an original still in the archive as itself; otherwise the copy is a document of its own (refined in review 28, which found taking a copy back from the Trash could refile the original). | `aCopyOfADocumentUndoneMeanwhileIsADocumentOfItsOwn` (undone, its file missing, its file elsewhere); `aCopyWhoseOriginalIsUndoneAsItGoesToTheTrashComesBackADocumentOfItsOwn` (each condition red-checked) | | 3 | 2 Minor | Before this change: queueing a filed document's own file in the archive (`arrumatorcli ingest <path>`) made a second document of it. | Refused, as the command says; docs/how-it-works.md says so. | `aDocumentsOwnFileInTheArchiveIsNotQueuedAsANewOne` (red-checked) | | 4 | 1 Nit | "First reading" named less than the guard covers: a document read again from Incoming is refused too. | Worded "reading in"; the reindex test's comment says "or undid". | — | ### Review 28 — the sixteenth commit | # | Severity | Finding | Fix | Kept by | |---|---|---|---|---| | 1 | 3 Major | Taking a copy back from the Trash, its original undone meanwhile, could collide with the original put back where the copy was; the job then read the undone original and filed it again, a second document. | Nothing comes back from the Trash. Once a copy is found one, kept with its job, it is handed over whatever the user does to the original meanwhile, as though the user did it after: a document no longer in the archive is not read again, and a file put at the copy's path is never taken for it (`isStill`, by what the job hashed), but queued in its own turn. A copy found one before a stop, still in Incoming, whose original is undone meanwhile is a document of its own, kept as no copy. | `aCopyInTheTrashIsHandedOverWhateverBecomesOfItsOriginalMeanwhile` (undone back where the copy was, its file removed, another file put in its place); `aCopyFoundBeforeAStopWhoseOriginalIsUndoneMeanwhileIsADocumentOfItsOwn` | | 2 | 3 Major | A stop once the copy was in the Trash, before its original's reading was queued, then an undo before the next start, recorded the copy as a file that disappeared, and left it in the Trash unrecorded. | The hand-over kept with the job is finished at the next start whatever the original became. | `aCopyInTheTrashAtAStopIsHandedOverThoughItsOriginalIsUndoneBeforeTheNextStart` | | 3 | 3 Major | Which originals take a copy had no test: waiting for the user, set aside after failing, left for later, and one left for later queued to be read again. | Tested. | `aCopyOfADocumentWaitingForTheUserOrSetAsideHasItReadAgain` (three statuses); `aCopyOfADocumentLeftForLaterStoppedOnceItsReadingWasQueuedSaysItIsReadAgain`; sixteen mutations, each failing a test | | 4 | 2 Minor | Before this change: `arrumatorcli ingest` of a file the user put into the archive, not read yet, or of one an earlier version set aside, sent it to the Trash as a copy. | No file in the archive is queued as an arrival; the command says so. | `aFileInTheArchiveIsNeverQueuedAsAnArrival` (a documen…
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
The first release run failed because every Vision text recognition on GitHub's
xcode-27runner threwTextRecognition.CRImageReaderError error 9. The first commit here found that, by making the OCR assertions print the extraction warnings. The runner is a virtual Mac, and Vision can't recognise text there on any device: forcing the CPU fails too, with an unknown error. Fixes:.enabledonly where a direct Vision probe reads text. Elsewhere they are skipped with the reason "needs Vision text recognition, which this machine cannot run". The probe calls Vision itself, notOCRService, so a defect in the app's OCR still fails them. They run on every physical Mac, as the push protocol's local gates do.scripts/verify.shlists skipped tests with their reasons.swift test --quiethid them. It now prints skips and totals on success and the whole log on failure.docs/how-it-works.mdanddocs/releasing.mdare updated.Acceptance criteria and proof
scripts/verify.sh --apppasses locally, where the OCR tests run.Test coverage
OCRServiceTests:Not covered: a Neural Engine compile failure on a physical Mac can't be provoked. The scripted recognizer stands in for it.
What changes for an installed app
Scans whose Neural Engine path fails are read on the CPU instead of losing their text. OCR trace steps gain a
devicefield per page. No stored data changes.Checklist
scripts/verify.sh --apppasses🤖 Generated with Claude Code