From 2d26df32fffefc73456eaab793a3c905139d51e5 Mon Sep 17 00:00:00 2001 From: Torlando <281092095+torlando-agent[bot]@users.noreply.github.com> Date: Mon, 21 Sep 2026 22:56:25 +0000 Subject: [PATCH] fix(nomadnet): partial refresh must not accumulate image records MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 Greptile P1 (Partial Images Accumulate): the round-1 fix appended fragment image records to the record list, so repeated refreshes of an image-bearing partial re-appended on each pass and the list grew until LIMIT_EXCEEDED permanently blocked that partial. Correct semantics (reference 1.4.3: detect_images runs only at full page load, never on partial refresh): copy the record list verbatim from the base — same records, same order, same compact indices, constant count across repeated refreshes. Fragment image blocks carry no record (image_index -1) and render as alt-text placeholders until the next full load, consistent with the loader being configured once per page. Test: native partial-core check now requires a constant image count across two refreshes and a placeholder fragment image; verified to fail against the accumulation-buggy build. --- lib/tdeck_ui/UI/LXMF/NomadNetCompactPage.cpp | 51 +++++++------------- tests/native/test_nomadnet_partial_core.cpp | 42 ++++++++++------ 2 files changed, 45 insertions(+), 48 deletions(-) diff --git a/lib/tdeck_ui/UI/LXMF/NomadNetCompactPage.cpp b/lib/tdeck_ui/UI/LXMF/NomadNetCompactPage.cpp index 468cd1c2..a791c831 100644 --- a/lib/tdeck_ui/UI/LXMF/NomadNetCompactPage.cpp +++ b/lib/tdeck_ui/UI/LXMF/NomadNetCompactPage.cpp @@ -462,14 +462,15 @@ PartialReplaceResult CompactPage::assign_replacing_partial( } // Image records are stable identity: the screen's decode slots and - // the loader entries are both keyed by the compact image index. - // Base images must therefore be re-copied in original order (same - // index), so a decoded image keeps rendering after a refresh. The - // reference detects images only at full page load — a partial - // refresh never scans the refreshed region for new images — so - // fragment images are appended after the base records (offset - // remap below) and render as alt-text placeholders: valid records, - // no loader entry, no fetch. + // the loader entries are both keyed by the compact image index, and + // the loader is configured once at full page load. The reference + // detects images only at full page load — a partial refresh never + // scans (or loads) images, neither in the retained body nor in the + // refreshed region — so the record list is copied verbatim from the + // base: same records, same order, same indices, and a constant + // count across repeated refreshes (no accumulation, no remap). + // Fragment image blocks therefore carry no record (image_index -1) + // and render as alt-text placeholders until the next full load. for (const auto& source : base._images) { if (_images.size() >= MAX_IMAGES) { clear(); @@ -490,24 +491,6 @@ PartialReplaceResult CompactPage::assign_replacing_partial( image.align = source.align; _images.push_back(image); } - for (const auto& source : fragment_page._images) { - if (_images.size() >= MAX_IMAGES) break; // the rest fall back - // to placeholders - ImageRecord image; - if (!append_view(fragment_page.image_alt(source), image.alt_offset, - image.alt_length) || - !append_view(fragment_page.image_url(source), image.url_offset, - image.url_length)) { - clear(); - return PartialReplaceResult::LIMIT_EXCEEDED; - } - image.width_kind = source.width_kind; - image.height_kind = source.height_kind; - image.width_value = source.width_value; - image.height_value = source.height_value; - image.align = source.align; - _images.push_back(image); - } struct CopyMaps { ExternalVector links; @@ -677,15 +660,15 @@ PartialReplaceResult CompactPage::assign_replacing_partial( block.partial_region_index = region_override >= 0 ? region_override : old.partial_region_index; if (old.image_index >= 0) { - // Base images were re-copied in order (identity), fragment - // images were appended after them (offset remap). - const std::size_t base_count = - (&source == &base) ? source._images.size() - : base._images.size(); - const std::size_t mapped = (&source == &base) + // Base records were copied verbatim (identity), so a + // retained image keeps its compact index. Fragment images + // have no record in the new page (a partial refresh never + // loads images) and fall back to -1. + const bool from_base = (&source == &base); + const std::size_t mapped = from_base ? static_cast(old.image_index) - : base_count + static_cast(old.image_index); - block.image_index = mapped < _images.size() + : _images.size(); // out of range -> placeholder + block.image_index = (from_base && mapped < _images.size()) ? static_cast(mapped) : -1; } if (old.table_index >= 0) { diff --git a/tests/native/test_nomadnet_partial_core.cpp b/tests/native/test_nomadnet_partial_core.cpp index a610eb54..382a6e34 100644 --- a/tests/native/test_nomadnet_partial_core.cpp +++ b/tests/native/test_nomadnet_partial_core.cpp @@ -260,8 +260,9 @@ int main() { preserved_form.fields()[0].value_length) == "user value"); // Image records must survive a partial refresh with stable identity: - // base images keep their compact index (decode slots and loader entries - // are keyed by it), fragment images append after the base records. + // the base list is copied verbatim (same records, same indices), the + // count stays constant across repeated refreshes (no accumulation), and + // fragment images render as placeholders (image_index -1). const auto img_source = parser.parse( R"(before `(base img`a.webp) @@ -280,11 +281,10 @@ after)"); img_base, 0, img_fragment, CompactPage::MAX_ARENA_BYTES); check("partial refresh preserves base image records and identity", img_replace == PartialReplaceResult::APPLIED && - img_candidate.images().size() == 2 && + img_candidate.images().size() == 1 && img_candidate.has_image_blocks()); - if (img_replace == PartialReplaceResult::APPLIED && - img_candidate.images().size() == 2) { - // Base image: same compact index (0), same alt/url. + if (img_replace == PartialReplaceResult::APPLIED) { + // Base image: same compact index, same alt/url. const auto& base_image = img_candidate.blocks()[1]; check("retained image block keeps its compact image index", base_image.type == BlockType::IMAGE && @@ -292,16 +292,30 @@ after)"); check("retained image record keeps its alt and url", img_candidate.image_alt(img_candidate.images()[0]) == "base img" && img_candidate.image_url(img_candidate.images()[0]) == "a.webp"); - // Fragment image: inserted at the region position (after the - // fragment text), appended to the record list after the base - // records (offset remap), valid record but no loader entry - // (rendered as placeholder). + // Fragment image: placeholder (no record, no fetch). const auto& frag_image = img_candidate.blocks()[3]; - check("fragment image is appended after base records with offset remap", + check("fragment image renders as a placeholder block", frag_image.type == BlockType::IMAGE && - frag_image.image_index == 1 && - img_candidate.image_alt(img_candidate.images()[1]) == "frag img" && - img_candidate.image_url(img_candidate.images()[1]) == "b.webp"); + frag_image.image_index == -1); + // Second refresh: the candidate is the new base; the record list + // must not accumulate (constant count), and the base image keeps + // its index. + const auto img_fragment2 = parser.parse( + R"(v2 +`(frag2 img`c.webp))"); + CompactPage img_candidate2; + const auto img_replace2 = img_candidate2.assign_replacing_partial( + img_candidate, 0, img_fragment2, CompactPage::MAX_ARENA_BYTES); + check("repeated partial refresh does not accumulate image records", + img_replace2 == PartialReplaceResult::APPLIED && + img_candidate2.images().size() == 1); + if (img_replace2 == PartialReplaceResult::APPLIED && + img_candidate2.images().size() == 1) { + check("base image keeps its index after the second refresh", + img_candidate2.has_image_blocks() && + img_candidate2.image_alt(img_candidate2.images()[0]) == + "base img"); + } } const auto region_source = parser.parse( "base `\n"