Learnt from: ariel-lindemann
Repo: crosspoint-reader/crosspoint-reader PR: 1133
File: lib/I18n/translations/finnish.yaml:92-92
Timestamp: 2026-02-23T22:11:42.181Z
Learning: In translation YAML files located in lib/I18n/translations/, literal percent signs (%) in strings (e.g., STR_HIDE_BATTERY, STR_GO_TO_PERCENT, STR_PERCENT_STEP_HINT) do not require escaping as %% and this convention is consistent across all language translations. Ensure that this pattern is followed for all YAML files in this directory to avoid introducing incorrect formatting markers in localized strings.

Learnt from: jpirnay
Repo: crosspoint-reader/crosspoint-reader PR: 1495
File: lib/I18n/translations/german.yaml:294-322
Timestamp: 2026-03-25T10:28:30.516Z
Learning: In crosspoint-reader, it is expected to include “pre-emptive” translation keys in non-English YAML files before the corresponding English keys exist (e.g., german.yaml ahead of en.yaml). When reviewing changes to files under lib/I18n/translations/*.yaml, do not flag missing corresponding English keys in non-English locales as an error if the repo’s gen_i18n.py fallback mechanism is intended to handle missing English keys safely until the pending PRs land.

Learnt from: KymAndriy
Repo: crosspoint-reader/crosspoint-reader PR: 1684
File: lib/I18n/translations/ukrainian.yaml:29-29
Timestamp: 2026-04-17T18:59:20.878Z
Learning: For Ukrainian (and other Slavic-language) translation YAML strings in lib/I18n/translations/*.yaml: if a format string contains a numeric placeholder produced via snprintf-style formatting (e.g., %zu) and runtime noun inflection is not feasible, place the relevant noun in genitive plural before the number in the string (e.g., "мереж %zu") rather than after it (e.g., "мережу %zu"). This avoids incorrect case forms that would otherwise result from the value affecting inflection (e.g., wrong forms like "мережу/мережі/мереж"). In such cases, do not flag the word order as incorrect.

Learnt from: whyte-j
Repo: crosspoint-reader/crosspoint-reader PR: 733
File: lib/I18n/I18nKeys.h:271-272
Timestamp: 2026-02-16T22:25:35.674Z
Learning: In the crosspoint-reader i18n system (lib/I18n/), missing translation keys in non-English YAML files are automatically filled with English strings at build time by the gen_i18n.py script. All generated string arrays (STRINGS_EN, STRINGS_ES, etc.) have identical lengths, so runtime array indexing is safe without per-string fallback logic. The system prints "INFO: '{key}' missing in {lang_code}, using English fallback" during code generation when this occurs.

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 1534
File: src/activities/boot_sleep/SleepActivity.cpp:0-0
Timestamp: 2026-04-05T10:43:16.171Z
Learning: In crosspoint-reader/crosspoint-reader (src/CrossPointSettings.h), CrossPointSettings::SLEEP_COVER_OVERLAY enum has values: OVERLAY_OFF=0, OVERLAY_WHITE=1, OVERLAY_GRAY=2, OVERLAY_BLACK=3 (as of commit 2471b5a in PR `#1534`). The default sleepCoverOverlay byte is 0 (OVERLAY_OFF). Do not use the old ordering WHITE=0/GRAY=1/BLACK=2/OFF=3 in future reviews.

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 1554
File: src/activities/boot_sleep/SleepActivity.cpp:40-63
Timestamp: 2026-04-02T15:35:02.228Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/boot_sleep/SleepActivity.cpp, SleepActivity::renderBitmapSleepScreen), CrossPointSettings::SLEEP_SCREEN_FILTER::FILTER_CONTRAST is intentionally handled implicitly: `hasGreyscale` is computed as `bitmap.hasGreyscale() && SETTINGS.sleepScreenFilter == FILTER_NONE`. When FILTER_CONTRAST is selected, `hasGreyscale` evaluates to false, which skips the 4-level greyscale rendering path and falls back to 1-bit B&W dithered rendering, producing a visibly higher-contrast result on bitmaps with greyscale data. LOGO and BLANK screens are unaffected by FILTER_CONTRAST by design — they contain no bitmap content where a contrast effect would be meaningful. Do not flag FILTER_CONTRAST as unimplemented or a no-op in future reviews.

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 1488
File: src/activities/home/HomeActivity.cpp:64-68
Timestamp: 2026-03-25T01:30:23.778Z
Learning: In crosspoint-reader/crosspoint-reader, `CrossPointState::pendingCoverGeneration` (bool, default false) and `CrossPointState::forceRenderCoverPath` (std::string, default "") are purely transient in-memory fields. They are NOT serialized in `JsonSettingsIO::saveState`/`loadState`. On every reboot they revert to their defaults, so there is no reboot-replay risk and no need to call `APP_STATE.saveToFile()` after consuming them in `HomeActivity::loadRecentCovers()`. Do not flag their in-memory consumption as a persistence bug.

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 1488
File: src/activities/reader/EpubReaderMenuActivity.cpp:39-45
Timestamp: 2026-04-09T20:05:11.820Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/reader/EpubReaderMenuActivity.cpp, EpubReaderMenuActivity::buildMenuItems), the `hasCoverForTheme` boolean (derived from `Storage.exists(coverPath)`) is intentionally used — not `!coverDisabled` — to choose between STR_HOME_COVER_ACTION_DISABLE and STR_HOME_COVER_ACTION_ENABLE in non-TIMEOUT global modes. The label reflects the action that will be performed: BMP exists → "Disable" (delete it); no BMP → "Enable" (generate it). Using `coverDisabled` would create confusing edge cases (e.g., coverDisabled=false with no BMP on disk would show "Disable" when there is nothing to disable). Do not flag this as a bug in future reviews.

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 1488
File: src/components/themes/lyra/Lyra3CoversTheme.cpp:39-40
Timestamp: 2026-03-25T01:15:06.744Z
Learning: In crosspoint-reader/crosspoint-reader (src/components/themes/lyra/Lyra3CoversTheme.cpp and other theme drawing functions), `COVER_DISABLED_MODE` (global setting) only prevents new cover *generation* on HOME entry to avoid blocking the main thread. It does NOT suppress the display of already-cached covers. Only the per-book `coverDisabled` flag (RecentBook::coverDisabled) controls whether a cached cover BMP is rendered. Do not flag theme draw paths for not checking COVER_DISABLED_MODE; checking only coverDisabled is correct by design.

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 1031
File: src/activities/reader/EpubReaderActivity.cpp:816-853
Timestamp: 2026-02-21T16:47:45.578Z
Learning: In src/activities/reader/EpubReaderActivity.cpp, the navigateToHref() function uses a fixed-size mini-stack (MAX_FOOTNOTE_DEPTH = 3) for saved positions rather than a dynamic stack. This is intentional to minimize RAM usage on ESP32-C3, which has limited memory. The design accepts that failed navigation at max depth may decrement footnoteDepth even when no push occurred, as a tradeoff for memory efficiency. Users exit by pressing Back three times maximum.

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 1488
File: src/activities/home/HomeActivity.cpp:73-77
Timestamp: 2026-04-09T19:53:18.381Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/home/HomeActivity.cpp, HomeActivity::loadRecentCovers and HomeActivity::render), `coverDisabled` must ONLY be cleared (set to false) at two explicit points: (1) after a successful `generateThumbBmp()` call in `loadRecentCovers`, and (2) after an explicit user "Enable cover" / "Generate cover" COVER_ACTION in EpubReaderActivity. Do NOT clear `coverDisabled` based on the mere existence of a cached cover BMP (neither in `loadRecentCovers` nor in `render`). A stale BMP on disk is not proof of user re-enable intent and must not override the per-book `coverDisabled` flag.

Learnt from: Tritlo
Repo: crosspoint-reader/crosspoint-reader PR: 1003
File: src/activities/reader/EpubReaderActivity.cpp:657-674
Timestamp: 2026-02-19T17:46:36.345Z
Learning: In src/activities/reader/EpubReaderActivity.cpp's renderContents() method, when uncached images exist, Phase 1 intentionally calls displayBuffer(forceFullRefresh) to perform a HALF_REFRESH (if needed), while Phase 2 intentionally calls renderer.displayBuffer() directly without forceFullRefresh. This is by design: Phase 1's refresh clears the screen properly to prevent ghosting, so Phase 2 can use a faster refresh mode for better performance. The grayscale anti-aliasing is handled separately after renderContents() via displayGrayBuffer().

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 1629
File: src/activities/home/HomeActivity.cpp:119-120
Timestamp: 2026-04-12T12:28:33.205Z
Learning: When reviewing code in this repository (C/C++ sources), set review comment severity according to this policy:
- Use **Major** (🟠) only for defects with realistic risk of crash, out-of-memory (OOM), invalid pointer dereference, data corruption, or other severe issues that are unlikely to be caught in casual/manual device testing.
- Use **Minor** or informational for UX gaps, logic edge-cases, style issues, or missing feature completeness that the author can verify (or has verified) through normal device use.
- Do **not** escalate severity to Major based on behavioral/UX observations alone; assume the author has already tested the feature on their own device and only treat issues as Major if they match the high-risk defect categories above.

Learnt from: znelson
Repo: crosspoint-reader/crosspoint-reader PR: 1447
File: src/activities/reader/EpubReaderActivity.cpp:642-660
Timestamp: 2026-03-20T19:51:12.696Z
Learning: In crosspoint-reader/crosspoint-reader (EpubReaderActivity.cpp, prepareSection), when `epub->getTocIndexForSpineIndex(currentSpineIndex)` returns a negative value (pre-TOC spine with no TOC entry), `prepareSection` skips the entire chapter walk and leaves `chapterPageInfo` unset. The status bar correctly falls back to `section->pageCount` in this scenario. This means the `pageTocIndex == -1` path inside the `render()` per-page TOC update block is handled correctly by simply leaving `chapterPageInfo` unchanged — no explicit clear is needed.

Learnt from: ngxson
Repo: crosspoint-reader/crosspoint-reader PR: 1016
File: src/activities/reader/EpubReaderActivity.cpp:138-146
Timestamp: 2026-02-22T19:13:22.166Z
Learning: In src/activities/reader/EpubReaderActivity.cpp, the EpubReaderMenuActivity result callback intentionally applies orientation changes (via applyOrientation) before checking result.isCancelled. This differs from other callbacks in the file because orientation is treated as an immediate setting that should persist even when the user cancels the menu, rather than a deferred action requiring confirmation.

Learnt from: Tritlo
Repo: crosspoint-reader/crosspoint-reader PR: 1003
File: src/activities/reader/EpubReaderActivity.cpp:657-674
Timestamp: 2026-02-19T17:46:26.871Z
Learning: In EpubReaderActivity.cpp, renderContents() behavior is intentional: if uncached images exist, Phase 1 should call displayBuffer(forceFullRefresh) to perform a HALF_REFRESH and clear ghosting; Phase 2 should call renderer.displayBuffer() without forceFullRefresh for a faster refresh. Grayscale anti-aliasing is applied separately after renderContents() via displayGrayBuffer(). This guidance applies specifically to this file and situation.

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 1488
File: src/activities/home/HomeActivity.cpp:144-145
Timestamp: 2026-03-28T11:14:39.035Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/home/HomeActivity.cpp, HomeActivity), `recentsLoaded` must NOT be reset in `onEnter()`. It defaults to `false` for new instances, but for retained instances it must stay `true` between HOME visits to prevent redundant cover regeneration on every entry. Cover generation is gated on `APP_STATE.pendingCoverGeneration` (set in `EpubReaderActivity::onExit()`) and `APP_STATE.forceRenderCoverPath` (set by Reader Menu cover actions) — these are the only authoritative triggers. Resetting `recentsLoaded` in `onEnter()` would cause generation to fire on every HOME visit (e.g., returning from Settings). Only `coverRendered` and `coverBufferStored` are correctly reset in `onEnter()`.

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 0
File: :0-0
Timestamp: 2026-03-28T20:57:09.493Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/home/HomeActivity.cpp, HomeActivity::onEnter), `coverRendered = false` and `coverBufferStored = false` ARE correctly reset in `onEnter()`. This was fixed in PR `#1488`. Do not flag their absence as a bug in future reviews.

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 0
File: :0-0
Timestamp: 2026-04-09T18:06:14.309Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/home/HomeActivity.cpp, HomeActivity::loadRecentCovers), the XTC generateThumbBmp() failure path uses master-style handling: it clears coverBmpPath by calling RECENT_BOOKS.updateBook(book.path, book.title, book.author, "") rather than setting coverDisabled=true. Only the EPUB path uses setCoverDisabled(true) on generateThumbBmp failure. Do not flag XTC failure→clear-bmpPath vs EPUB failure→coverDisabled as an inconsistency; they are intentionally different by design.

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 1534
File: src/activities/boot_sleep/SleepActivity.cpp:177-183
Timestamp: 2026-03-29T19:50:44.877Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/boot_sleep/SleepActivity.cpp, SleepActivity::getBookOverlayInfo), when reading totalPages from the TXT index cache (index.bin), the code must validate the cache header: check the magic number (0x54585449 / "TXTI") and version fields before trusting totalPages. This mirrors TxtReaderActivity::loadPageIndexCache(). If validation fails, totalPages stays 0 and the overlay falls back to "Page X" (no total) via STR_OVERLAY_READING_PROGRESS_NO_TOTAL. Do not flag this validation as unnecessary in future reviews.

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 0
File: :0-0
Timestamp: 2026-04-09T18:26:10.532Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/home/HomeActivity.cpp, HomeActivity::loadRecentCovers), the XTC generateThumbBmp() failure path uses master-style handling: it clears coverBmpPath by calling RECENT_BOOKS.updateBook(book.path, book.title, book.author, "") rather than setting coverDisabled=true. Only the EPUB path uses setCoverDisabled(true) on generateThumbBmp failure. The reason is intentional by design: XTC reader has no reader menu COVER_ACTION (no "Enable/Generate cover" option), so setting coverDisabled=true on XTC failure would permanently block regeneration with no user-accessible recovery path. Clearing the BMP path instead allows HOME to fall back to the classical placeholder, and a future successful generateThumbBmp() call will repopulate it naturally. Do not flag XTC failure→clear-bmpPath vs EPUB failure→coverDisabled as an inconsistency in future reviews.

Learnt from: znelson
Repo: crosspoint-reader/crosspoint-reader PR: 1447
File: src/activities/reader/EpubReaderActivity.cpp:642-660
Timestamp: 2026-03-20T19:51:12.696Z
Learning: In crosspoint-reader/crosspoint-reader (EpubReaderActivity.cpp), orphan/post-TOC spines always inherit a non-negative `tocIndex` from the previous chapter via `BookMetadataCache`. Therefore `getTocIndexForSpineIndex` and `getTocIndexForPage` (which falls back to `getTocIndexForSpineIndex` when `tocBoundaries` is empty) will never return -1 for post-TOC spines. The only case where `pageTocIndex == -1` can occur is a genuine pre-TOC spine (e.g. the very first spine) that has no TOC entry at all and no predecessor to inherit from.

Learnt from: znelson
Repo: crosspoint-reader/crosspoint-reader PR: 1582
File: lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp:497-501
Timestamp: 2026-04-16T06:13:29.082Z
Learning: In `lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp`, when the image consumes the empty block's accumulated vertical spacing, the reset of `currentTextBlock`'s `BlockStyle` to a fresh `BlockStyle` (with only `alignment` set) is intentionally correct. `getCombinedBlockStyle(..., CombineAxis::Vertical)` initializes `BlockStyle result = child`, so horizontal properties on the empty block are never read — the block-style stack provides horizontal context for the next element. Only the vertical margins/padding from the empty block are read during the merge, and the reset zeroes exactly those. Preserving horizontal insets from the empty block would add complexity for zero behavioral difference. Do not flag this reset as dropping inherited horizontal style in future reviews.

Learnt from: ngxson
Repo: crosspoint-reader/crosspoint-reader PR: 1218
File: src/activities/ActivityManager.cpp:254-265
Timestamp: 2026-02-27T22:49:59.600Z
Learning: In this codebase, assertions are always enabled (no NDEBUG). Use assert() to crash on programmer errors and surface logic bugs during development and in production builds. Do not rely on asserts for runtime error handling; they should enforce invariants that must always hold. Keep asserts side-effect free and inexpensive, and avoid relying on them for user-visible failures. Include <cassert> where appropriate and document the invariant being tested.

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 1245
File: lib/Epub/Epub/Section.cpp:277-308
Timestamp: 2026-03-02T10:14:16.036Z
Learning: Guideline: Strengthen serialization::readString to defend against unbounded growth when reading from disk data. Implement and enforce a maximum allowed length (e.g., a configured or reasonable constant) and validate the incoming length before resizing or allocating. Audit all call sites (e.g., BookMetadataCache, TextBlock, KOReaderCredentialStore, Section cache readers) to ensure they do not rely on unbounded len-based resizing. If the readString API must remain, add internal safeguards (bounds checks, length validation, and error handling) so per-call-site validations are not required. Ensure Section cache files remain versioned (SECTION_FILE_VERSION) and parameter mismatches invalidate caches, but do not rely on unsafe allocations; prefer safe, bounded reads with explicit errors on invalid data.

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 1488
File: src/activities/home/HomeActivity.cpp:92-95
Timestamp: 2026-03-28T11:06:29.611Z
Learning: When reviewing crosspoint-reader code, avoid flagging a missing `renderer.displayBuffer()` call immediately after `GUI.drawPopup()` / `BaseTheme::drawPopup()`: `BaseTheme::drawPopup()` already calls `renderer.displayBuffer()` before returning, so the popup is guaranteed to be flushed to the e-ink panel before subsequent blocking work begins. Conversely, do not require a `renderer.displayBuffer()` call after `fillPopupProgress()`; it intentionally does not flush, so intermediate progress-bar updates may not appear unless the update granularity warrants an explicit flush elsewhere.

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 988
File: lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp:649-661
Timestamp: 2026-02-19T12:17:05.421Z
Learning: In ChapterHtmlSlimParser.cpp, when computing footnote word indices in endElement() for footnote links, the wordIndex must be cumulative across the 750-word mid-paragraph flush boundary. The correct calculation is: `wordIndex = wordsExtractedInBlock + currentTextBlock->size()`, not just `currentTextBlock->size()`. This ensures footnotes attach to the page containing their actual anchor word, even after layoutAndExtractLines(false) has extracted and removed earlier words from the block.

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 1245
File: lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp:137-150
Timestamp: 2026-03-02T21:18:25.387Z
Learning: In ChapterHtmlSlimParser.cpp, pendingAnchorId is a single string (not a vector) because EPUB footnote targets are virtually always block-level elements (<aside>, <p>, <div>) that trigger startNewTextBlock() and flush the anchor before overwrite can occur. Multiple consecutive inline IDs would resolve to the same completedPageCount anyway, and vector allocation overhead is unnecessary for ESP32-C3. Only inline IDs in practice are footnote back-references (<a id="ref1">), which are never navigation targets.

Learnt from: znelson
Repo: crosspoint-reader/crosspoint-reader PR: 1454
File: lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp:101-105
Timestamp: 2026-03-21T19:30:33.664Z
Learning: In `lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp`, the `blockStyleStack` is a fixed-capacity array (`MAX_BLOCK_STYLE_DEPTH`, currently 8). On overflow, `pushBlockStyle()` only logs an error and increments `blockStyleStackSize` without writing a new entry; `getAccumulatedBlockStyle()` returns the deepest stored accumulated entry as a fallback. This is intentional — real EPUBs are expected to have ≤ 8 block-style nesting levels. Inline styles use a `vector`-based stack and can nest more deeply. The overflow threshold or container type can be revisited if 8 proves insufficient in practice. Do not flag the overflow fallback behavior as a bug in future reviews.

Learnt from: jpirnay
Repo: crosspoint-reader/crosspoint-reader PR: 1646
File: lib/KOReaderSync/ChapterXPathIndexerState.h:80-80
Timestamp: 2026-04-13T09:12:18.349Z
Learning: In `lib/KOReaderSync/ChapterXPathIndexerState.h`, `shouldSkipText(len)` is intentionally a fast structural gate only (skipDepth, zero-length, outside body). Both callers (`ChapterXPathForwardMapper.cpp` and `ChapterXPathReverseMapper.cpp`) check `isWhitespaceOnly(text, len)` separately and later, because whitespace-only nodes may still require partial processing before being discarded — in particular, the forward mapper accumulates `codepointsInBodyTextNode` for body-level whitespace text nodes. Do not suggest merging `isWhitespaceOnly` into `shouldSkipText` in future reviews.

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 1245
File: lib/Epub/Epub/Section.cpp:238-246
Timestamp: 2026-03-02T09:58:47.539Z
Learning: In EPUB section files, typical sections contain approximately 20 footnote/note anchors plus a few sub-chapter anchors (max ~30-50 total) even in large chapters. While the code tracks all HTML id attributes, realistic anchor counts per section remain well under 255, making uint8_t sufficient for 99% of EPUBs, though uint16_t provides extra safety margin for edge cases.

Learnt from: znelson
Repo: crosspoint-reader/crosspoint-reader PR: 1447
File: lib/Epub/Epub/Section.cpp:115-117
Timestamp: 2026-03-20T19:43:16.969Z
Learning: In crosspoint-reader/crosspoint-reader, `Section::buildTocBoundariesFromFile()` and `Section::buildTocBoundaries()` are always called on freshly constructed `Section` objects (created via `std::make_unique<Section>(...)` in `prepareSection`). The `tocBoundaries` member vector is therefore always default-initialized (empty) before either builder runs — no explicit `tocBoundaries.clear()` is needed inside these methods. Do not flag missing clears as a bug in future reviews.

Learnt from: znelson
Repo: crosspoint-reader/crosspoint-reader PR: 1413
File: lib/EpdFont/EpdFont.cpp:34-36
Timestamp: 2026-03-17T15:27:17.468Z
Learning: In crosspoint-reader/crosspoint-reader, `EpdFont::getGlyph()` falls back to `getGlyph(REPLACEMENT_GLYPH)` (U+FFFD) before returning `nullptr`. All fonts in this project are guaranteed to include a U+FFFD replacement glyph, so the `!glyph` null branch in `EpdFont::getTextBounds()` (and similar rendering paths) is unreachable in practice. Do not flag stale-state issues in that branch (e.g., leftover `lastBaseAdvanceFP`/`lastBaseTop` after a null glyph) as bugs in future reviews.