review-and-fix

name: review-and-fix description: Deep analysis and iterative fixing of a Rust source file. Finds logic errors, lock/concurrency issues, and simplification opportunities, then fixes them one by one until the file is clean.

Deep-analyze the specified file, find logic errors, memory/lock issues, and simplification opportunities, and fix them iteratively until no problems remain.

Usage

/review-and-fix <file_path>

Examples

/review-and-fix src/mqtt-broker/src/subscribe/buckets.rs
/review-and-fix src/mqtt-broker/src/subscribe/directly_push.rs

Execution Flow

Each round follows this sequence until nothing left to fix:

1. Read

Fully read the target file. Read related files as needed (callers, struct definitions it depends on) to understand context.

2. Analyze

Check in priority order:

Logic Errors (must fix)

Asymmetric data structure operations: add writes N indexes, remove only cleans N-1
offset/commit semantics: committing after push failure causes message loss
Key collisions: separator choice produces identical keys for different inputs

Concurrency/Lock Issues (must fix)

DashMap entry(), get(), get_mut() return Ref/RefMut that hold shard locks — not released during .await
RwLock read lock held during .await blocks write lock
Fix: .clone() the data to drop the guard before awaiting; or store Arc<T>

Simplification (apply judiciously)

Repeated get_mut + else { insert } → entry().or_default()
Redundant else { return x } → remove the else
Two-step let x = ...; let x = match x { Some(v) => v, None => return } → let Some(x) = ... else { return }
Nested if !condition { ... } → if condition { continue }
Duplicate import lines → merge
Temporary flag variables (let mut failed = false; ... if !failed { commit() }) → early return

Comments (keep lean)

Remove redundant or obvious comments; the code should speak for itself
Keep only comments that explain a non-obvious why (an invariant, a subtle ordering, a footgun)
Do not over-comment — fewer, higher-signal comments beat many noisy ones

Test Cases (assess and complete)

Coverage: identify core logic paths and edge cases that are not tested; add focused tests for the gaps (the pure decision functions and the bug-prone branches first)
Conciseness: if tests are redundant or overlapping (one case is a subset of another), slim them down — tests should be few, focused, and high-signal, not exhaustive duplication
Prefer testing the pure/extractable logic directly over standing up heavy mocks for orchestration glue

Naming (align names with behavior)

Function names: does the name describe what the function actually does? Rename misleading or vague names (e.g. a get_* that mutates, a *_switch that only computes)
File / module names: does the file name match its content and responsibility? Flag/rename when it has drifted
When renaming, update every reference (callers, imports, mod declarations) and re-run cargo check. Be conservative with widely-used public names — only rename when the current name is genuinely misleading, not for taste

What NOT to do

Do not refactor correct code just to be "more Rusty"
Do not introduce new abstractions or traits
Do not change public API signatures (unless there is a bug, or a name is genuinely misleading — then rename and update all call sites)
Do not add unnecessary comments

3. Fix

Only fix issues you are certain about — do not guess
After each fix, run cargo check -p <crate> to verify compilation
For core logic changes, run the relevant unit tests

4. Loop

After each round of fixes, re-analyze the file to confirm nothing was missed. Stop only when you can clearly state: "no logic errors, no lock issues, no worthwhile simplification remaining, names match behavior, test coverage adequate and focused, comments lean."

Output Format

Start each round by stating what problems were found
After fixing, explain what changed and why
On the final round, explicitly state "no issues, stopping"
Do not output meaningless progress descriptions