review: batch-end quality round - #11
Merged
Merged
Conversation
Delete a restated comment, tighten a stale doc comment to match the current return type, and move a misplaced doc comment onto the function it actually documents.
Move the nav const block above its first use, glue StabilityConf's Default impl to the struct, switch to let-else and lazy defaulting where the match/eager form obscured the same logic, group the test module's use imports, and drop redundant std:: qualification where the type is already imported.
Inline a single-use type and a single-use private method, drop a redundant self-rebind, consolidate the four request-text traversals (blocklist scan, inbound-text view, mask application, DLP redaction) behind one shared helper, and factor the repeated poisoned-mutex lock into one function in the memory store.
Remove the hand-rolled epoch-millis computation in new_request_id; reuse gw_state::epoch_millis, already depended on and already called elsewhere in this crate. No behaviour change.
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
1 participant
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.
Batch-end quality round across the Rust workspace, the Go control plane and the web client: style walkthrough, comment budget, simplify lenses, and a loc-justify pass with adversarial verification.
Removed
crates/handler/src/lib.rs:new_request_id's hand-rolled epoch-millis.gw_state::epoch_millis()is the sameSystemTime::now().duration_since(UNIX_EPOCH)sequence,gw-stateis already a dependency of this crate, and this crate already calls it elsewhere.Kept, after verification
control-plane/internal/auth/password.go:VerifyPassword. The candidate's argument that everyPasswordHashwriter goes throughHashPasswordis factually right, but the conclusion does not follow:VerifyPasswordis deliberately written to be constant-time against the dummy-hash path inauth.go, and that property is the point of the function, not incidental volume.Style, layout and comments
crates/config/src/lib.rs: movedimpl Default for StabilityConfdirectly below its struct and relocated the six serde-default helpers after the impl, matching the conventionTokenRateConfandAlertsConfalready follow in the same file.crates/state/src/store.rs: dropped a needless explicit lifetime on the mutex helper (elision produces the identical signature) and trimmed its doc to one line, keeping the WHY (it recovers a poisoned lock rather than panicking because every critical section here is infallible).Gates
cargo fmt --all -- --check: cleancargo clippy --workspace --all-targets -- -D warnings: 0 warnings, 0 errorscargo test --workspace: everytest result:line ok, 0 failedcd control-plane && GOWORK=off go vet ./... && GOWORK=off go test ./...: passnpm --prefix web test && npm --prefix web run build: 2 tests pass, build okCargo.lock,control-plane/go.mod,control-plane/go.sum,web/package-lock.jsonunmodifiedNet: -77 prod, 0 test.