fix(inference): make pruning bounded and retry-safe - #1773
Open
Ryanchen911 wants to merge 3 commits into
Open
Ryanchen911 wants to merge 3 commits into
Ryanchen911 wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved pruning-index retention, budget accounting, and non-positive-limit handling issues remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR fixes pruning budget handling, restores default pruning limits, and preserves active inferences.
Changes:
- Adds default inference and PoC pruning limits.
- Accumulates pruning counts across epochs.
- Adds active-status preservation and regression tests.
File summaries
| File | Summary |
|---|---|
inference-chain/x/inference/types/params.go |
Adds default pruning limits; non-positive limits still require validation or guarding. |
inference-chain/x/inference/keeper/pruning.go |
Updates budget accounting and status handling; retains unresolved index, error-accounting, logging, and zero-limit concerns. |
inference-chain/x/inference/keeper/pruning_test.go |
Adds backlog and status-preservation regression coverage. |
Review details
Suppressed comments (2)
inference-chain/x/inference/keeper/pruning.go:367
PruneEpochruns fromEndBlock, andPruneinvokes it for each pruner, so this adds an Info log for every epoch-pruner invocation on normal blocks with eligible data. That creates sustained log volume during routine operation; this diagnostic should be Debug-level.
p.Logger.LogInfo("PruneEpoch called", types.Pruning, "epoch", currentEpochIndex, "prunesLeft", prunesLeft, "list", p.List.GetName())
inference-chain/x/inference/types/params.go:228
- These limits are still accepted as zero or negative by
EpochParams.Validate(only the threshold is checked), andPruneEpochcompares the budget only after removing an item. A governance update or legacy params record withInferencePruningMax == 0therefore still removes one item per call, violating the advertised maximum; the new defaults do not protect those configurations. Reject non-positive limits or guard before enteringPruneEpoch.
InferencePruningEpochThreshold: 2, // Number of epochs after which inferences can be pruned
InferencePruningMax: 1000, // Maximum number of inferences to prune per block
PocPruningMax: 1000, // Maximum number of PoC items to prune per block
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Author
|
@copilot review |
…vation This commit fixes three critical issues in the generic Pruner implementation that were blocking issue gonka-ai#1499 and causing pruning failures: 1. **Missing prunedCount accumulation (lines 413-421 in pruning.go)** - Root cause: prunedCount was declared but never accumulated across epochs - Impact: Every epoch received full PruningMax budget instead of sharing it - Result: First block deleted all items (batch delete), and smaller epochs were skipped over by the marker, leaving data permanently orphaned - Fix: Added 'prunedCount += prunedForEpoch' and early return when limit hit - Verification: TestPrunerMultiEpochBacklogBudget now passes, properly spreading 12,600 deletions across 13 blocks at 1000/block 2. **Missing default pruning limits (params.go lines 226-227)** - Root cause: InferencePruningMax and PocPruningMax were absent from DefaultEpochParams(), causing them to default to 0 - Impact: No pruning occurred at all (0 items per block = infinite backlog) - Fix: Added both fields with value 1000 to default params - This was the primary blocker preventing any pruning from working 3. **Missing status-based pruning exemption (pruning.go lines 205-212)** - Root cause: Remover function directly deleted inferences without checking their status field - Impact: VOTING and STARTED status inferences were pruned when they should have been preserved (active validation state) - Fix: Added status check in Remover - skip deletion for VOTING/STARTED but still remove from pruning index to avoid infinite re-checks - Verification: TestPruningStatusPreservation now passes Additional changes: - Added TestPrunerMultiEpochBacklogBudget based on bonujel's 2026-07-30 reproduction case (6 epochs, 12,600 items, varying sizes) - Added debug logging in PruneEpoch to track per-epoch pruning progress These fixes unblock PR gonka-ai#1499 (InferenceValidationDetails gradual pruning) and address the core pruning infrastructure issues identified in issue gonka-ai#1223. Refs: gonka-ai#1223, gonka-ai#1499 Co-Authored-By: Claude <noreply@anthropic.com>
Ryanchen911
force-pushed
the
ryan/fix-pruner-prunedcount
branch
2 times, most recently
from
September 17, 2026 06:48
17c1544 to
269445f
Compare
This was referenced Sep 17, 2026
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.
Summary
PruningMaxacross all eligible epochs in a blockVOTING/STARTEDinferences to the current epoch instead of orphaning themTests
FINISHED, and are removed on a later retry without stale indexesgo test ./x/inference/keeper ./x/inference/types ./app/upgrades/...This is the prerequisite for the
InferenceValidationDetailspruner in #1499.Refs #1223 and #1499.