mirror of
https://github.com/torlando-tech/pyxis.git
synced 2026-10-23 06:12:25 +00:00
fix(nomadnet): partial refresh must not accumulate image records
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.
This commit is contained in:
1 parent
d39a2b75f1
commit
2d26df32ff
2 files changed
+45
-48
No files matched your search
@@ -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<int16_t> 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<std::size_t>(old.image_index)
|
||||
: base_count + static_cast<std::size_t>(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<int16_t>(mapped) : -1;
|
||||
}
|
||||
if (old.table_index >= 0) {
|
||||
|
||||
@@ -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 `<same`base-default>\n"
|
||||
|
||||
Reference in new issue
Block a user