Skip to content

Fix PR #50 review finding: Doppler drop memory churn - #51

Merged
daedalus merged 2 commits into
masterfrom
ccr-503d6e26-3ijog5
Oct 1, 2026
Merged

daedalus merged 2 commits into
masterfrom
ccr-503d6e26-3ijog5

Conversation

@daedalus

@daedalus daedalus commented Oct 1, 2026 •

Copy link
Copy Markdown
Owner

Addresses the Copilot thread on #50, plus the Copilot review on this PR.

Bug (core/power_doppler.py)
Fixed 2¹⁵-key LRU of dropped keys: a corpus cycle larger than that evicted each key just before it returned → no revisit seen → horizon never widened → zero frames. Reachable with unlimited --max-corpus.

Fix
Remember the bottom-k dropped keys by crc32_ieee (k = max_dropped): a fixed subset of any cycle, never churned out, never empty. Max-heap with lazy stale skipping; rebuilt (deduplicated) when it doubles past k. The first iteration of this PR used a halving crc threshold, which emptied out on all-odd-crc cycles (Copilot review); replaced.

Tests

  • test_regression_cycle_beyond_drop_memory_still_scores: 200 seeds, 8-key memory → frames close, horizon ≤ 4×cycle, memory ≤ 8.
  • test_regression_masked_out_cycle_keeps_a_witness: cycle of odd-crc keys only, 2-key memory → frames close (failed on the threshold version).
  • test_adversarial_drop_memory_is_bottom_k_crc: remembered set equals the exact k smallest crcs.
  • test_adversarial_redropped_key_keeps_heap_bounded: drop/revisit churn keeps heap ≤ 2k.
  • 102 passed (test_power_doppler.py, test_schedules.py); ruff + lizard clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Rvj2gdMAa4HA8MGHULfDJE

The fixed 2^15-key LRU forgot every dropped key once the corpus cycle
outgrew it, so revisits were never seen and frames never closed.
Remember a crc32-sampled subset instead, halving the sample on
overflow: sampled keys are never churned out, memory stays bounded.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Rvj2gdMAa4HA8MGHULfDJE
@sourcery-ai

This comment was marked as resolved.

@daedalus
daedalus marked this pull request as ready for review October 1, 2026 14:56
Copilot AI balanced review requested due to automatic review settings October 1, 2026 14:56
sourcery-ai[bot]

This comment was marked as off-topic.

This comment was marked as resolved.

A halving crc threshold subset emptied out when every key had an odd
crc, so no revisit widened the horizon again. Keep the k smallest
crc32s instead: a fixed, never-empty subset of any cycle. Use the
repo's crc32_ieee; test with a cycle of masked-out keys.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Rvj2gdMAa4HA8MGHULfDJE

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The bounded implementation addresses the reported churn failure with focused regression coverage.

Review effort: Balanced
Findings: None

Resolved since last review (3)

@daedalus
daedalus merged commit 8eec4dd into master Oct 1, 2026
1 check passed
@daedalus
daedalus deleted the ccr-503d6e26-3ijog5 branch October 1, 2026 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants