fix(argus): unbreak the lib test, and clear the crate's hidden clippy debt - #35
Merged
Conversation
… debt THE REGRESSION FIRST. #34 dropped `candle_core::D` from siglip.rs once `SoftmaxInplace` replaced `softmax_last_dim` -- but a TEST still used `D::Minus1`, so `cargo test -p ffai-argus` has not compiled on master since. `cargo clippy --lib` lints the LIB TARGET ONLY and never builds `#[cfg(test)]` code, so every clippy check stayed green while the test target was broken. The test now says `candle_core::D` and the lib import stays dropped. That is the gap worth remembering: CI runs `--lib` for the six library crates and `--all-targets` only for the binaries, so a lint-driven edit can break tests invisibly. WHY THERE WERE 84 FINDINGS. `-D warnings` aborts a crate's COMPILE, so its dependents are never linted at all. ffai-argus sat behind ffai-core and ffai-media and had not been checked in a long time; clearing those two is what made it visible. Nothing here is new debt. NINE REAL FIXES: decode.rs manual `pos` counter -> a range loop preprocess.rs `for k in 0..n` indexing `row` -> `iter_mut().take().enumerate()` preprocess.rs `clip8` -> `const fn` engine.rs `with_manifest_dir` -> `const fn` engine.rs redundant `as_deref_mut`, redundant `&` on a `&str` siglip.rs unneeded `return` text.rs two `iter_mut()` loops -> `&mut` A 43-SITE RENAME, not a silence. The 44 `used_underscore_binding` findings were `let _t = Instant::now()` paired with `prof::add("...", _t)`: the underscore claims the binding is unused while the value goes straight into the profiler. Clippy was right. `_t` -> `t` across siglip.rs and text.rs, 87 occurrences. EIGHT ALLOWS, each mirroring one ffai-mercury already documents for the same reason -- manual_let_else, doc_markdown, needless_pass_by_value, too_many_lines, too_long_first_doc_paragraph, option_if_let_else, inline_always, many_single_char_names. None can change behaviour. Both CI clippy commands now exit 0 ON LINUX, which is where CI runs and where three earlier findings were hiding from a Windows-only check. ffai-bench and ffai-mercury turned out clean once argus compiled, so this closes the chain. Gates: ffai-argus 42 lib + 27 oracle -- preprocess_oracle pins the rewritten Lanczos weights against PIL and engine_oracle runs 83 s of real inference, which are the two that would catch the decode-loop and resampler changes. ffai-core 43, ffai-mercury 136 + 27. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… missed
`harden.yml`'s Clippy job runs THREE blocking commands, not two. The third is
`-p ffai-carmenta -p ffai-diana --all-targets`, and I had been reproducing only
the first two -- which is why every local check came back clean while CI stayed
red. My error, not another masked layer.
Note the third uses `--all-targets` where the six library crates use `--lib`.
That is the difference that would have caught the `D` regression, and it is why
two genuinely dead methods show up here and nowhere else.
MOSTLY MECHANICAL, via `cargo clippy --fix`, all semantics-preserving and each
one checked in the diff rather than trusted:
22 cast_lossless `x as f64` -> `f64::from(x)`
5 unnecessary_to_owned
2 manual_div_ceil `(a + b - 1) / b` -> `a.div_ceil(b)`
1 range_plus_one `out[1..1 + n]` -> `out[1..=n]`
1 needless_question_mark `Ok(x?)` -> `x`
+ use_self, map_unwrap_or, cloned_instead_of_copied,
needless_option_as_deref, collapsible_else_if
table.rs `.max(1).min(SIDE)` -> `.clamp(1, SIDE)`. Safe because these are
`usize`: the NaN divergence that makes `manual_clamp` a real
question for float code cannot arise.
TWO DEAD METHODS, FLAGGED RATHER THAN DELETED. `Svtr::w` and
`Graph::resolve`/`get` are referenced nowhere, including tests and examples --
`dead_code` only became visible when the compile chain stopped aborting earlier.
They are the shaped-weight accessor and the graph walker's scope-resolution
half. Deleting another author's loader internals is not a lint fix, so they
carry `#[allow(dead_code)]` and a note saying the call is theirs.
ONE FALSE POSITIVE. `if_same_then_else` in `suppress.rs`: two `true` arms that
document the §13 harvest bypass and the shipped §8.157 order guard. They accept
for unrelated reasons and collapsing them would erase which one fired -- the
same call ffai-mercury's manifest already makes for this lint.
All THREE CI clippy commands now exit 0 on Linux.
Gates: ffai-carmenta 42 lib + 9 integration, ffai-diana 76 lib + 8 integration.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 participants
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.
The regression, first
#34 dropped
candle_core::Dfromsiglip.rsonceSoftmaxInplacereplacedsoftmax_last_dim— but a test still usedD::Minus1, socargo test -p ffai-argushas not compiled on master since it merged.cargo clippy --liblints the lib target only and never builds#[cfg(test)]code, so every clippy check stayed green while the test targetwas broken. CI runs
--libfor the six library crates and--all-targetsonlyfor the binaries, which is the gap that let it through.
Why ffai-argus had 84 findings
-D warningsaborts a crate's compile, so its dependents are never linted atall.
ffai-argussat behindffai-coreandffai-media; clearing those two iswhat made it visible. None of this is new debt — it had simply not been checked
in a long time.
ffai-benchandffai-mercuryturned out clean once arguscompiled, so this closes the chain.
Nine real fixes
decode.rsposcounter → range looppreprocess.rsfor k in 0..nindexingrow→iter_mut().take().enumerate()preprocess.rsclip8→const fnengine.rswith_manifest_dir→const fnengine.rsas_deref_mut; redundant&on a&strsiglip.rsreturntext.rsiter_mut()loops →&mutA 43-site rename, not a silence
The 44
used_underscore_bindingfindings werelet _t = Instant::now()pairedwith
prof::add("...", _t)— the underscore claims the binding is unused whilethe value goes straight into the profiler. Clippy was right.
_t→tacrosssiglip.rsandtext.rs, 87 occurrences.Eight allows
Each mirrors one
ffai-mercuryalready documents for the same reason:manual_let_else,doc_markdown,needless_pass_by_value,too_many_lines,too_long_first_doc_paragraph,option_if_let_else,inline_always,many_single_char_names. None can change behaviour.Gates
Both CI clippy commands exit 0 on Linux — where CI runs, and where three
earlier findings were hiding from a Windows-only check.
ffai-argus42 lib + 27 oracle.preprocess_oraclepins the rewrittenLanczos weights against PIL and
engine_oracleruns 83 s of real inference —the two that would catch the decode-loop and resampler changes.
ffai-core43,ffai-mercury136 + 27🤖 Generated with Claude Code