Re-prime the prior gate on evict/revisit reload (#118) - #132
Merged
Conversation
Extend reloadEvictedTile at both call sites (offline + live node) to re-apply Chart+Reference prior after survey restore; add regression test.
Extract seedNewTile's whole prior rung (Chart exact -> Reference exact -> #115 containment-checked cross-level fallback, predicted-only, warn-and-continue) into file-local primePriorLayersForTile and call it from reloadEvictedTile BEFORE the survey restore -- prior first so the finer survey-derived predicted depth overwrites where measured data exists, the same order as first touch. Previously a tile whose predicted surface came only from the prior returned from eviction with the blunder gate (and #59 slope correction) silently OFF. Re-primes are logged for auditability. Regression test evicts a reference-gated tile and revisits with a deep blunder at a never-surveyed cell; verified to FAIL with the re-prime disabled.
reloadEvictedTile now re-primes the revisited tile from prior_store_dir first (single-tile exact-level Chart-then-Reference lookups -- not a whole-window prime, which would lazy-create neighbor nodes against the very budget doing the evicting), then the draft restore overwrites where survey data exists. The revisit loop also runs when only a prior store is configured: prior-primed tiles are clean and evictable without draft persistence, and previously could never be re-primed. Throttled INFO on re-prime, throttled WARN + continue-ungated on a prior read error. The two 'deferred as #118' comments now describe the shipped behaviour.
…ing, widened revisit gate (#118)
…prior read failure (#118) Address the two pre-push must-fixes on the live revisit gate path plus the re-prime-failure observability suggestion: - Reload-before-add (must-fix): the revisit reload loop ran AFTER addSoundings, so a false-deep blunder in the FIRST revisit batch of an evicted prior-only tile was settled ungated (Node::insert accepts anything while predicted_depth_ is NaN) before the reload re-primed the gate. Move the loop before addSoundings and key it off gridIndicesForSoundings -- the same influence-radius-expanded window addSoundings touches -- mirroring the offline importer's reload-before-add. Failed reloads are dropped AFTER the add. - Keep-evicted-on-prior-read-failure (must-fix): a thrown prior loadWindow was caught and the draft-empty branch fell through to 'return true', so the caller erased the evicted marker and the prior-only tile ran ungated for the rest of the session with no retry. Track prior_ok and return it from both the draft-empty and survey-restore branches so a prior read failure keeps the tile evicted for retry. - Observability (suggestion): the re-prime-failure WARN now names the tile and throttles tighter (1 s) than the success INFO (30 s) so distinct failing tiles are less likely to be collapsed, and states the (now real) retry contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…118) Offline analog of the live-node prior-read-failure fix. primePriorLayersForTile returned only 'did I prime a cell', not 'did the windowed load succeed', so a thrown prior read on revisit (caught internally, returns false) followed by a successful survey restore left ImportAccumulator::reloadEvictedTile returning true -- the caller erased the evicted_ marker and the tile's prior-only cells ran ungated for the rest of the run with no retry. Add an optional read_ok out-param that reports whether the load threw, and return it from reloadEvictedTile so a prior read failure keeps the tile evicted (caller drops it, protecting the intact on-disk survey surface) for a later revisit to retry -- the same drop-and-retry already used for survey read errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add ImportEviction.PriorReadFailureOnRevisitKeepsTileEvicted: survey a cell of a reference-gated tile, evict it, corrupt the reference tile so the revisit prior re-prime read throws (survey restore still succeeds), then revisit with a deep blunder at a different never-surveyed cell. The blunder must not settle -- the tile is kept evicted and its ungated revisit soundings dropped. Exercises the shared reload logic that the live CubeBathymetryNode::reloadEvictedTile mirrors (the ROS node class is not unit-test-exposed -- behind main(), no header/library target). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR closes #118 by ensuring tiles that are evicted and later revisited regain their prior-seeded predicted surface (and therefore blunder gating and slope correction) before new soundings are accumulated, in both the offline importer and the live node.
Changes:
- Extracts the import-time prior priming logic into a shared helper and reuses it during
ImportAccumulator::reloadEvictedTileso revisited tiles are re-gated before survey restoration. - Updates the live node’s revisit path to re-prime priors and to reload evicted tiles before
addSoundings, with retry semantics on prior read failure. - Adds regression tests covering the evict→revisit gate retention and the “prior read failure keeps tile evicted” retry behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cube_bathymetry/test/test_import_eviction.cpp | Adds regression tests for evict/revisit prior-gate retention and retry-on-prior-read-failure behavior. |
| cube_bathymetry/src/store_import.cpp | Introduces primePriorLayersForTile() helper; re-primes priors on reload and propagates prior-read success to control eviction retry semantics. |
| cube_bathymetry/src/cube_bathymetry_node.cpp | Re-primes priors on revisit; reloads evicted tiles before addSoundings; keeps tiles evicted on prior read failure to enforce retry behavior. |
| cube_bathymetry/include/cube_bathymetry/store_import.h | Documents the updated reloadEvictedTile semantics (prior-first, then survey restore). |
| .agent/work-plans/issue-118/plan.md | Adds the implementation plan for issue #118. |
| .agent/work-plans/issue-118/progress.md | Records progress/review notes and rationale for design decisions and must-fix resolutions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+683
to
+686
| // cross-level Reference fallback resample. Warn-and-continue on any load error | ||
| // (the tile simply runs ungated; a prior read failure must never drop | ||
| // soundings). Shared by seedNewTile (first touch) and reloadEvictedTile (#118 | ||
| // revisit re-prime); @p context names the caller in the audit/error lines. |
Comment on lines
+781
to
+782
| std::cerr << "import_bag: could not prior-seed tile on " << context << ": " | ||
| << e.what() << " (no prior gate for this tile)" << std::endl; |
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
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.
Closes #118.
Summary
Closes the last gap in the prior-gate chain (#89 → #96 → #115 → #119/#91): a tile whose predicted surface came only from the reference/chart prior used to return from eviction with the blunder gate — and, since #59, live slope correction — silently OFF, because the reload path restored the Survey layer only.
seedNewTile's whole prior rung (Chart exact-level → Reference exact-level → the Blunder gate: seed reference prior from whichever level is present, not only the survey level #115 containment-checked cross-level fallback; predicted-only; warn-and-continue) is extracted into file-localprimePriorLayersForTile()and now also runs inImportAccumulator::reloadEvictedTile— before the survey restore, so the finer survey-derived prediction still wins (same order as first touch andon_configure).CubeBathymetryNode::reloadEvictedTilere-primes the revisited tile fromprior_store_dirvia single-tile exact-level Chart-then-Reference lookups (deliberately not a whole-window prime — that would lazy-create neighbor nodes against the very budget doing the evicting). Evicted tiles are now reloaded beforeaddSoundings, so even the first batch on a revisited tile is gated (round-1 review must-fix). The revisit loop also runs in prior-only deployments (no draft store) — prior-primed tiles are clean and evictable regardless of draft persistence.reloadEvictedTilepath has no node-level test harness — its semantics are covered via the import-path regression test and the Live chart-prior seeding: prime the predicted surface in the live node to gate false-deep detections (live #89 equivalent) #91 helper tests.Test plan
ReferenceOnlyTileEvictRevisitKeepsGate: a reference-gated tile is surveyed on one cell, evicted by spread batches, then revisited with a −150 m blunder at a different, never-surveyed cell — the blunder must be rejected and the original survey must survive the round-trip. Discrimination verified: with the reload re-prime disabled, the test fails (pre-fix bug reproduced).PriorReadFailureKeepsTileEvictedcovering the retry semantics./review-code(deep tier, 2 rounds): round 1 changes-requested (2 live-path must-fixes, both fixed), round 2 approved / Ship: recommended, 0 must-fix.Authored-By:
Claude Code AgentModel:
Claude Fable 5