test(scaling): prove live CSR allocation budget after ranking merge - #719
seonghobae wants to merge 1 commit into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@opencode-agent address Fresh exact-current reconciliation + GREEN handoff for issue #612 / Draft #719 after valid RED. Immediately before any write refetch source head, live protected The branch is 13 protected-main commits behind with exactly one unique fail-first test file. Reconcile current main non-destructively first; preserve RED evidence from CI run Implement the minimum GREEN on the reconciled head:
Do not widen to |
|
@opencode-agent address Reacquire the bounded source-writer lease for Draft #719 / issue #612 only if a final refetch still shows source head Fresh compare is diverged: this branch has exactly one unique fail-first allocation test path and is 17 protected-main commits behind, merge base The exact RED proved two real resource-contract mismatches: under a 32-byte declared CSR budget the implementation requests a 64-byte uint64 capacity, and accepted rankings create list→uint64 temporaries in addition to live CSR buffers. Implement only the bounded GREEN:
Do not weaken the resource ceiling, make process-wide memory claims, modify ranking/stationary-distribution arithmetic, dependencies/workflows/version/release, or canonical #604 docs. Keep Draft; final exact-head review/Ready/merge returns to the maintainer loop. |
|
@opencode-agent address Fresh replacement reconciliation/GREEN lease for issue #612 / Draft #719 after protected Implement the minimum GREEN already defined by #719/#612: make the declared live fixed-width CSR budget true or explicitly narrow/rename the contract to what is actually guaranteed; do not weaken a live-memory ceiling while retaining stronger wording. Remove or fully budget the per-ranking list→uint64 temporary, include growth/overlap of simultaneously live uint64 buffers, preserve at-most- Keep Rust stationary/ranking arithmetic unchanged and do not widen into |
|
Superseded by GREEN implementation PR (constraint/allocation/uncertainty ownership). Closing RED draft. |
Post-integration fail-first correction for #612
Protected
mainat branch creation and final pre-PR refetch:640de7dd39d944749b92b1a88bafa8e1787a0847.PR #692 has merged, but fresh protected-main inspection shows that the accepted implementation still does not prove the memory contract documented by
_rankings_to_csr(). Logical(flat_count + start_count) * 8is bounded, while internal NumPy buffers grow with a minimum/geometric capacity (max(..., 8, old_capacity * 2)) and each ranking is copied throughnp.asarray(ranking_items, dtype=np.uint64). Therefore actual fixed-width allocations can exceed a tiny declared budget even when final logical CSR payload fits, and the per-ranking conversion introduces an additional unbudgeted uint64 temporary beside live CSR arrays.This Draft adds only two allocation-boundary REDs:
These tests instrument actual NumPy allocation/conversion requests and call
_rankings_to_csr()directly; setup/import/fixture failure is not valid RED.Minimum GREEN
np.asarray(list, dtype=np.uint64)temporary while preserving boundedn + 1consumption._top1_to_csrremains issue security: bound top-1 loser iterables before CSR materialization #632 and is not part of this correction.Do not weaken the resource limit, create a process-wide memory claim, change dependencies/workflows/version/release, or touch canonical #604 docs. Keep Draft through RED→GREEN. This is required before #612 may be closed despite #692's merge.