From ce14c9d53be53f9235bba97f04b6f7b92c3c7dc4 Mon Sep 17 00:00:00 2001 From: John Date: Wed, 23 Sep 2026 01:01:01 -0500 Subject: [PATCH] Agent identified fixes 2 --- README.md | 17 ++++++++++---- TODO.md | 27 ++++++++++++++++++---- epub_to_chapters.py | 18 ++++++++++++++- include/Config.h | 6 ++++- include/Reader.h | 3 ++- src/Library.cpp | 4 ++-- src/Reader.cpp | 15 ++++++++++-- src/main.cpp | 42 +++++++++++----------------------- tests/test_epub_to_chapters.py | 11 +++++++++ 9 files changed, 98 insertions(+), 45 deletions(-) diff --git a/README.md b/README.md index a4893e3..a0e9642 100644 --- a/README.md +++ b/README.md @@ -112,8 +112,9 @@ with EPUB styles, punctuation, and repeated invented names: The output directory must be empty unless `--overwrite` is supplied. Copy the resulting `Example Book` directory into `/books` on the card. A -section may be up to 1.5 MiB. The converter writes `CONVERSION_REPORT.txt` in -the book directory. It records any unsupported non-ASCII characters left after +section and its `.rsvp` sidecar may each be up to 1 MiB. The converter writes +`CONVERSION_REPORT.txt` in the book directory. It records any unsupported +non-ASCII characters left after normalizing common punctuation, ligatures, accents, whitespace, fractions, and ornamental breaks. Review that report before copying the book: transliterate or replace reported characters deliberately, or defer the book until a @@ -123,8 +124,10 @@ printable ASCII. The converter also writes a compact `
.rsvp` file alongside every section. Its versioned, delta-encoded records preserve italic and bold EPUB markup—including semantic tags, CSS classes, and inline CSS—and reading-timing -metadata. During conversion, the desktop-only English dictionary marks each -word not in that dictionary. The first ten appearances of each marked word in +metadata. The fingerprint covers the source `.txt` bytes, before the device +applies display-only punctuation normalization. During conversion, the +desktop-only English dictionary marks each word not in that dictionary. The +first ten appearances of each marked word in the book, plus its first appearance in every section, use a 3× delay. Later appearances use a 2× delay. The reader uses the strongest applicable delay, so sentence and paragraph pauses are never compounded with unfamiliar-word @@ -228,6 +231,10 @@ pressing the touch-edge Home button begins at the saved paragraph. It validates the saved section against its `TOC.txt` before opening it, then falls back to the first available book if the card has changed. +When continuous reading reaches the end of a section, it opens the next +section's title card and pauses. Press the touch-edge Home button to begin that +section. + The physical power switch only disconnects the battery. USB power is sufficient for development and flashing. The reader intentionally has no Wi-Fi, Bluetooth, cloud account, or synchronization feature. @@ -248,7 +255,7 @@ cloud account, or synchronization feature. ## Limits of the first MVP -- It reads sections up to 1.5 MiB from `/books//
.txt`, in the +- It reads sections up to 1 MiB from `/books//
.txt`, in the order declared by that book's `TOC.txt`. - The built-in display font is best for printable ASCII text. UTF-8 text files are accepted, but accented/non-Latin characters need a Unicode font addition diff --git a/TODO.md b/TODO.md index 0bbc590..258194a 100644 --- a/TODO.md +++ b/TODO.md @@ -16,13 +16,32 @@ profile that preserves the current cadence. Add a `--check` mode that reports section and sidecar sizes, filename issues, and unsupported characters without writing output. Offer optional splitting at -paragraph boundaries for sections that exceed the device's 1.5 MiB file limit. +paragraph boundaries for sections that exceed the device's 1 MiB file limit. ## Memory-limit hardware validation -Stress-test the advertised maximum section and sidecar sizes on real hardware, -record PSRAM headroom, and lower the supported limit if token indexing or -sidecar metadata leaves insufficient memory for reliable reading. +Stress-test the 1 MiB section and sidecar limits on real hardware, record PSRAM +headroom, and lower the supported limit if token indexing or sidecar metadata +leaves insufficient memory for reliable reading. Include adversarial short-word +and dense-sidecar inputs, not just ordinary prose. + +## Reading continuity preference + +Keep the current default of pausing on every chapter title, but add an optional +auto-continue mode for readers who want uninterrupted reading across sections. + +## RSVP orientation point + +Offer an optional focal-letter/orientation-point treatment that subtly marks a +consistent letter within each displayed word. Keep the current centered plain +word rendering as the default and make the feature work with italic and bold +fonts. + +## Library diagnostics and import validation + +Add a device diagnostics screen for SD-card, TOC, missing-section, sidecar, +and memory-limit errors. Extend converter preflight to report the same issues, +including oversized tokens, before a book is copied to the card. ## Add anti-aliased reader fonts diff --git a/epub_to_chapters.py b/epub_to_chapters.py index 3a4d689..030c1fc 100644 --- a/epub_to_chapters.py +++ b/epub_to_chapters.py @@ -122,7 +122,8 @@ SIDECAR_HEADER = struct.Struct("<4sBBQI") CSS_RULE = re.compile(r"([^{}]+)\{([^{}]*)\}") CSS_CLASS = re.compile(r"\.([A-Za-z_-][A-Za-z0-9_-]*)") WORD_LOOKUP_KEY = re.compile(r"^[^A-Za-z]*([A-Za-z]+(?:['-][A-Za-z]+)*)[^A-Za-z]*$") -MAX_SECTION_BYTES = 1500 * 1024 +MAX_SECTION_BYTES = 1024 * 1024 +MAX_TOKEN_BYTES = 256 * 1024 MAX_SECTION_NAME_BYTES = 200 TIMING_NONE = 0 @@ -333,6 +334,16 @@ def lookup_word_key(token): return match.group(1).lower() if match else None +def validate_token_lengths(text): + """Reject a malformed section containing an impractically long token.""" + + for word_number, match in enumerate(re.finditer(r"\S+", text), start=1): + if len(match.group().encode("utf-8")) > MAX_TOKEN_BYTES: + raise ValueError( + f"token {word_number} exceeds the {MAX_TOKEN_BYTES} byte limit" + ) + + def load_english_dictionary(): """Load the desktop-only English dictionary required for timing analysis.""" @@ -1404,6 +1415,11 @@ def convert_epub(epub_path, output_dir, overwrite=False): for title, chapter in prepared_sections: text_bytes = chapter["text"].encode("utf-8") + try: + validate_token_lengths(chapter["text"]) + except ValueError as exc: + print(f"Error: {title!r} contains an invalid token: {exc}") + sys.exit(1) sidecar_bytes = style_sidecar_bytes( chapter["text"], chapter["style_records"] ) diff --git a/include/Config.h b/include/Config.h index 91b6561..36f0615 100644 --- a/include/Config.h +++ b/include/Config.h @@ -22,7 +22,11 @@ constexpr uint8_t kButtonPrevious = 12; constexpr uint8_t kButtonNext = 16; constexpr char kBooksDirectory[] = "/books"; -constexpr size_t kMaxBookBytes = 1500 * 1024; +// Source sections and their optional RSVP sidecars are loaded into memory. +constexpr size_t kMaxSectionBytes = 1024 * 1024; +// A single token can be a long URL or identifier, but should never consume a +// material fraction of the reader's working memory. +constexpr uint32_t kMaxTokenBytes = 256 * 1024; constexpr uint16_t kDefaultWpm = 300; constexpr uint16_t kMinimumWpm = 100; diff --git a/include/Reader.h b/include/Reader.h index 9cf9171..be7d277 100644 --- a/include/Reader.h +++ b/include/Reader.h @@ -5,7 +5,7 @@ struct Token { uint32_t offset; - uint16_t length; + uint32_t length; uint32_t paragraph; uint8_t trailingLineBreaks; }; @@ -39,6 +39,7 @@ class Reader { uint32_t index_ = 0; size_t nextStyleToken_ = 0; bool sidecarValid_ = false; + uint64_t sourceFingerprint_ = 0; bool loadSidecar(const std::vector& sidecar); void seekStyleToken(); diff --git a/src/Library.cpp b/src/Library.cpp index 110a5b4..061d070 100644 --- a/src/Library.cpp +++ b/src/Library.cpp @@ -44,7 +44,7 @@ bool Library::loadSidecar(const String& chapterPath, sidecarPath += ".rsvp"; File file = SD.open(sidecarPath, FILE_READ); if (!file || file.isDirectory() || file.size() == 0 || - static_cast(file.size()) > config::kMaxBookBytes) { + static_cast(file.size()) > config::kMaxSectionBytes) { file.close(); return false; } @@ -227,7 +227,7 @@ String Library::chapterName(const String& chapterPath) const { bool Library::loadFile(const String& path, String& contents) const { File file = SD.open(path, FILE_READ); if (!file || file.isDirectory() || file.size() == 0 || - static_cast(file.size()) > config::kMaxBookBytes) { + static_cast(file.size()) > config::kMaxSectionBytes) { return false; } contents = file.readString(); diff --git a/src/Reader.cpp b/src/Reader.cpp index 5731012..d70fe03 100644 --- a/src/Reader.cpp +++ b/src/Reader.cpp @@ -4,6 +4,8 @@ #include #include +#include "Config.h" + namespace { constexpr uint8_t kStyleItalic = 0x01; @@ -120,6 +122,9 @@ String normalizePunctuation(const String& text) { } // namespace bool Reader::load(const String& book, const std::vector& sidecar) { + // The sidecar describes the exact bytes on the card. Keep its fingerprint + // separate from the display normalization applied for tokenization. + sourceFingerprint_ = sourceFingerprint(book); book_ = normalizePunctuation(book); tokens_.clear(); styleTokens_.clear(); @@ -148,7 +153,13 @@ bool Reader::load(const String& book, const std::vector& sidecar) { lineBreaks = 0; } if (whitespace && inWord) { - tokens_.push_back({wordStart, static_cast(i - wordStart), + const uint32_t wordLength = i - wordStart; + if (wordLength > config::kMaxTokenBytes) { + tokens_.clear(); + paragraphStarts_.clear(); + return false; + } + tokens_.push_back({wordStart, wordLength, static_cast(paragraphStarts_.size() - 1), 0}); inWord = false; } @@ -167,7 +178,7 @@ bool Reader::loadSidecar(const std::vector& sidecar) { sidecar[2] != 'V' || sidecar[3] != 'P' || (sidecar[4] != kSidecarVersion1 && sidecar[4] != kSidecarVersion2) || sidecar[5] != 0 || - readUint64Le(sidecar, 6) != sourceFingerprint(book_)) { + readUint64Le(sidecar, 6) != sourceFingerprint_) { return false; } const uint8_t sidecarVersion = sidecar[4]; diff --git a/src/main.cpp b/src/main.cpp index 8cdf000..d71e63e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -23,8 +23,6 @@ bool displayIsOff = false; bool displayIsDimmed = false; uint32_t lastInteractionMs = 0; uint32_t nextWordMs = 0; -uint32_t lastPersistMs = 0; -bool stateDirty = false; TouchDrvCSTXXX touch; bool touchAvailable = false; bool touchPlayPauseRequested = false; @@ -87,21 +85,9 @@ bool released(Button& button) { return wasPressed && button.stable == HIGH; } -void markStateDirty() { stateDirty = true; } - -void persist(bool force = false) { - const uint32_t currentWord = reader.currentIndex(); - if (state.wordIndex != currentWord) { - state.wordIndex = currentWord; - stateDirty = true; - } - constexpr uint32_t kProgressCheckpointMs = 30UL * 1000UL; - if (!stateDirty || (!force && millis() - lastPersistMs < kProgressCheckpointMs)) { - return; - } +void persistState() { + state.wordIndex = reader.currentIndex(); settings.save(state); - stateDirty = false; - lastPersistMs = millis(); } void setReadingContext(uint8_t progressPercent) { @@ -144,7 +130,6 @@ bool loadChapter(const String& path) { state.bookPath = path; state.wordIndex = 0; reader.restoreAtParagraphStart(0); - markStateDirty(); readerView = ReaderView::kChapterTitle; playing = false; return true; @@ -187,6 +172,8 @@ void drawPicker() { } void openMenu() { + // Opening the menu stops playback, so it is a manual pause checkpoint. + if (playing) persistState(); playing = false; menuSelection = 0; screen = Screen::kMenu; @@ -246,8 +233,7 @@ void applyPicker() { } else { loadChapter(pendingChapterPath); } - markStateDirty(); - persist(true); + persistState(); returnToReader(); } @@ -263,7 +249,9 @@ void togglePlayback() { } else { playing = !playing; } - persist(true); + // Playing progress is intentionally not checkpointed. Resume is paragraph + // based, so writing at each pause avoids needless NVS wear. + if (!playing) persistState(); if (playing) nextWordMs = millis() + reader.delayMs(state.wpm); } @@ -388,7 +376,7 @@ void wakeDisplay() { } void enterDeepSleep() { - persist(true); + persistState(); board.displayOff(); // GPIO12 is RTC-capable on ESP32-S3. A press (active low) wakes the reader. esp_sleep_enable_ext0_wakeup(static_cast(config::kButtonPrevious), 0); @@ -421,12 +409,10 @@ void handleInput() { state.wpm = state.wpm > config::kMinimumWpm + config::kWpmStep ? state.wpm - config::kWpmStep : config::kMinimumWpm; - markStateDirty(); showCurrentWord(); } else { navigatePreviousPaused(); } - persist(true); } if (released(next)) { wakeDisplay(); @@ -443,12 +429,10 @@ void handleInput() { state.wpm = state.wpm + config::kWpmStep < config::kMaximumWpm ? state.wpm + config::kWpmStep : config::kMaximumWpm; - markStateDirty(); showCurrentWord(); } else { navigateNextPaused(); } - persist(true); } } } // namespace @@ -509,8 +493,7 @@ void setup() { const uint32_t savedWord = state.wordIndex; reader.restoreAtParagraphStart(savedWord); state.wordIndex = reader.currentIndex(); - stateDirty = sanitizedSettings || state.wordIndex != savedWord; - persist(true); + if (sanitizedSettings || state.wordIndex != savedWord) persistState(); readerView = ReaderView::kChapterTitle; showReaderScreen(); lastInteractionMs = millis(); @@ -531,11 +514,12 @@ void loop() { playing = false; showReaderScreen(); } - persist(); + // Reaching a chapter boundary pauses automatically; save that new + // chapter before waiting for the reader to continue. + persistState(); lastInteractionMs = millis(); } else { showCurrentWord(); - persist(); nextWordMs = millis() + reader.delayMs(state.wpm); } } diff --git a/tests/test_epub_to_chapters.py b/tests/test_epub_to_chapters.py index 1fff5cb..24bd54e 100644 --- a/tests/test_epub_to_chapters.py +++ b/tests/test_epub_to_chapters.py @@ -12,6 +12,7 @@ from epub_to_chapters import ( STYLE_ITALIC, TIMING_DOUBLE, TIMING_TRIPLE, + MAX_TOKEN_BYTES, add_unfamiliar_word_timing, decode_styled_text, document_root, @@ -24,6 +25,7 @@ from epub_to_chapters import ( parse_document, source_fingerprint, stylesheet_class_styles, + validate_token_lengths, write_style_sidecar, write_conversion_report, ) @@ -66,6 +68,15 @@ class DisplayTextNormalizationTests(unittest.TestCase): self.assertLessEqual(len(name.encode("utf-8")), 200) +class TokenValidationTests(unittest.TestCase): + def test_allows_a_token_at_the_configured_byte_limit(self): + validate_token_lengths("a" * MAX_TOKEN_BYTES) + + def test_rejects_a_token_larger_than_the_configured_byte_limit(self): + with self.assertRaises(ValueError): + validate_token_lengths("a" * (MAX_TOKEN_BYTES + 1)) + + class ConversionReportTests(unittest.TestCase): def test_reports_remaining_non_ascii_characters_by_generated_file(self): with tempfile.TemporaryDirectory() as temporary_directory: