fix: add max param to prune signals cmd

[MasterPtato]

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[railway-app]

🚅 Deployed to the rivet-pr-4190 environment in rivet-frontend

Service	Status	Web	Updated (UTC)
website	😴 Sleeping (View Logs)	Web	Feb 12, 2026 at 10:29 pm
frontend-cloud	❌ Build Failed (View Logs)	Web	Feb 12, 2026 at 10:22 pm
frontend-inspector	❌ Build Failed (View Logs)	Web	Feb 12, 2026 at 10:22 pm
mcp-hub	✅ Success (View Logs)	Web	Feb 12, 2026 at 10:21 pm
ladle	❌ Build Failed (View Logs)	Web	Feb 12, 2026 at 10:21 pm

[MasterPtato]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• fix: add max prunes to pruner #4191
• fix: add max param to prune signals cmd #4190 👈 (View in Graphite)
• fix: reduce metrics aggregator cardinality #4183
• fix: docker cache issues, dashboards #4181
• main

---

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

PR Review: Add max parameter to prune signals command

Summary

This PR adds a max_per_txn parameter to the prune signals command and makes several type improvements to parallelization parameters. The changes allow better control over transaction sizes when pruning signals, which is important for managing database load.

✅ Positive Aspects

1. Type Safety Improvement: Changing parallelization from u128 to u16 is a good improvement. The validation already limited it to < 1024, so u16 (max 65535) is more appropriate and prevents impossible values.

2. Consistent Parameter Handling: The max_per_txn parameter is properly threaded through all layers:

   • CLI command (signal.rs:41)
   • Database trait (debug.rs:70)
   • Implementation (kv/debug.rs:1818)

3. Early Exit Logic: The implementation correctly exits the loop early when max_per_txn is reached (kv/debug.rs:1877-1881), preventing oversized transactions.

4. Grafana Dashboard Fix: The change from rivet_gasoline_workflow_total to rivet_gasoline_workflow_sleeping appears to fix a metric query (though the PR description doesn't mention this).

⚠️ Issues & Concerns

1. Missing Default Value for max_per_txn

// signal.rs:85
parallelization.unwrap_or(1),
max_per_txn,  // ⚠️ Passed as Option<usize>, no default

Issue: Unlike parallelization which has a sensible default of 1, max_per_txn is passed as None by default. This means the pruning operation could create unbounded transaction sizes when the parameter isn't specified.

Recommendation: Add a default value or document why unbounded transactions are acceptable:

max_per_txn.or(Some(1000)),  // or another sensible default

2. Incomplete PR Description

The PR description template isn't filled out. Missing:

• What problem does this solve?
• Why is max_per_txn needed?
• What testing was performed?
• Why was the Grafana dashboard metric changed?

3. Logic Concern in Early Exit

// kv/debug.rs:1877-1881
if let Some(max_per_txn) = max_per_txn {
    if signal_ids.len() >= max_per_txn {
        return anyhow::Ok(());
    }
}

Question: When this early exit occurs, current_signal_id is set to the last signal processed. On the next iteration, will this skip the signal that triggered the limit? The byte manipulation on line 1839 (*b = 255) suggests it moves to the next key, but this should be verified.

Also: After early exit, line 1909 appears to continue processing, but the stream is exhausted. The outer loop logic should be verified.

4. Type Cast Pattern

// kv/debug.rs:1409, 1418, 1450, 1452
let chunk_size = u128::MAX / parallelization as u128;
for i in 0..parallelization as u128 {

This pattern is repeated 3 times. While correct, consider extracting this logic into a helper function to reduce duplication and potential for bugs.

5. Missing Test Coverage

The PR doesn't mention any tests. For a database pruning operation with new parameters, tests should verify:

• Behavior with max_per_txn set
• Behavior with max_per_txn as None
• Correct signal counting and iteration
• Edge cases (empty results, single signal, exactly max_per_txn signals)

6. Undocumented Grafana Change

-rivet_gasoline_workflow_total
+rivet_gasoline_workflow_sleeping

This appears to be a separate bug fix that should either:

• Be in a separate commit with explanation
• Be documented in the PR description

🔍 Performance Considerations

Positive: The max_per_txn parameter is exactly what's needed for controlling transaction size and preventing database overload during large pruning operations.

Concern: Without a default value, users might unknowingly create very large transactions that could:

• Lock the database for extended periods
• Cause memory issues
• Fail due to transaction size limits

🔒 Security Considerations

No security concerns identified. The parameter is validated through clap's type system and the implementation doesn't introduce any vulnerabilities.

📝 Recommendations

1. Add a sensible default for max_per_txn (e.g., 1000 or 10000 based on your database characteristics)
2. Fill out the PR description explaining the motivation and testing
3. Add unit tests covering the new parameter
4. Verify the early exit logic doesn't skip signals incorrectly
5. Document the Grafana change or split it into a separate commit
6. Consider adding a warning if max_per_txn is None and a large number of signals match

Overall Assessment

The core change is sound and addresses a real need for controlling transaction sizes during pruning. However, the lack of a default value and unclear early-exit behavior are concerns that should be addressed before merging. The type improvements (u128 → u16) are excellent.

---

Review generated for PR #4190 by Claude Code