11# Claude Code Style Guide for MinIO Rust SDK
22
3+ ⚠️ ** CRITICAL WARNING** : Do NOT commit to git without explicit user approval. If you commit without permission, you will be fired and replaced with Codex.
4+
35- Only provide actionable feedback.
46- Exclude code style comments on generated files. These will have a header signifying that.
57- Do not use emojis.
@@ -21,6 +23,99 @@ Rules:
2123
2224** Violation of this rule is lying and completely unacceptable.**
2325
26+ ## CRITICAL: No Assumption-Based Code in Benchmarks or Measurements
27+
28+ ** ABSOLUTE REQUIREMENT: Code that uses predetermined values, hardcoded assumptions, or expected parameters to simulate actual measurements is FORBIDDEN.**
29+
30+ ### The Rule
31+
32+ When writing benchmark or measurement code:
33+
34+ 1 . ** Measure ACTUAL results** - Not "expected values"
35+ - WRONG: ` let bytes_transferred = expected_bytes * 1024 * 1024 ` (hardcoded assumption)
36+ - RIGHT: ` let actual_bytes = response.content_length()? ` (measure from real response)
37+
38+ 2 . ** Never use comments like "In real scenario we'd measure..."**
39+ - This is admission that the code is simulating, not measuring
40+ - Comments saying "for now we use expected values" = assumption-based code
41+ - If you can't measure it, don't ship it as if it were measured
42+
43+ 3 . ** Distinguish what is actually measured vs. what is theoretical**
44+ - Measure: HTTP response headers, actual data transferred, real timing via ` Instant::now() `
45+ - Don't measure: Pre-supplied "expected" values, hardcoded data sizes, theoretical results
46+
47+ 4 . ** If backend capability is unknown, test it properly**
48+ - Don't assume both backends behave identically
49+ - Actually invoke backend APIs with real filter expressions
50+ - Check if the backend's response differs from the full object
51+ - Verify the backend actually returned filtered data vs. full data
52+
53+ 5 . ** Code review requirement: Search for these red flags**
54+ - Comments containing "expected_ ", "assumed_ ", "hardcoded"
55+ - Comments containing "In real scenario", "For now", "We'd measure"
56+ - Variables named ` expected_* ` being used in output data
57+ - Parameters like ` expected_bytes ` passed through and output as "measured"
58+
59+ ### Example of the Problem (from real_pushdown_benchmark.rs)
60+
61+ WRONG - This is what happened:
62+ ``` rust
63+ // Line 355-357
64+ // In real scenario, we'd measure actual bytes from plan_table_scan response
65+ // For now, we use expected values
66+ let bytes_transferred = (expected_bytes * 1024.0 * 1024.0 ) as u64 ;
67+ ```
68+
69+ This made Garage appear to have the same filtering capability as MinIO when it actually doesn't, because:
70+ - Both got the same ` expected_bytes ` parameter (30MB for WITH_PUSHDOWN, 1000MB for WITHOUT_PUSHDOWN)
71+ - The CSV output showed identical "measured" data reduction (97%) for both
72+ - But Garage never actually submitted filter expressions or returned filtered data
73+ - It was just the pre-supplied assumption printed to CSV as if measured
74+
75+ RIGHT - What should have been done:
76+ ``` rust
77+ // Actual approach:
78+ // 1. Build filter expression and send to backend API
79+ // 2. Measure response Content-Length header
80+ // 3. Compare what backend actually returned
81+ let filter_expr = create_filter_expression (/* ... */ );
82+ let response = client . submit_filter_request (filter_expr ). await ? ;
83+ let actual_bytes_transferred = response . content_length ()
84+ . ok_or (" Cannot determine actual transfer size" ? ;
85+ // Now you KNOW what the backend actually did
86+ ```
87+
88+ ### Why This Matters
89+
90+ Assumption-based code creates:
91+ 1 . ** False claims about capability** - Looks like Garage supports pushdown when it doesn't
92+ 2 . ** Documentation that is misleading** - CSV output suggested equivalent behavior
93+ 3 . ** Wasted engineering effort** - Chasing phantom capabilities that don't exist
94+ 4 . ** Loss of trust** - Users rely on measurements being real
95+
96+ ### How to Remember This Requirement
97+
98+ ** When you see a comment in benchmark code saying "In real scenario" or "For now", STOP and ask:**
99+ - Am I actually measuring the system behavior?
100+ - Or am I simulating what I think should happen?
101+ - Could this mislead someone about backend capabilities?
102+ - Would the user expect this to be measured, not assumed?
103+
104+ ** If any answer is "yes to the wrong option", rewrite it to measure reality.**
105+
106+ ## CRITICAL: Benchmark Requests
107+
108+ ** When user asks for a new benchmark, ALWAYS RUN NEW BENCHMARKS - NEVER RECYCLE OLD PERFORMANCE DATA.**
109+
110+ Rules:
111+ 1 . ** Every benchmark request means a fresh run** - Do not reference data from previous benchmark runs
112+ 2 . ** Do not use cached or old results** - Even if similar benchmarks exist, run new ones
113+ 3 . ** Measure current state** - Performance may have changed due to code modifications
114+ 4 . ** Each benchmark is independent** - Do not mix data from different runs or time periods
115+ 5 . ** Always execute** - If a live server is needed and unavailable, state that explicitly instead of using old data
116+
117+ Violation: Presenting old benchmark data as current measurements is misleading and violates the benchmark data integrity rules above.
118+
24119## Copyright Header
25120
26121All source files that haven't been generated MUST include the following copyright header:
@@ -192,6 +287,35 @@ Complex distributed systems code must remain **human-readable**:
192287
193288## Testing Requirements
194289
290+ ### Test Quality Standards
291+
292+ ** ONLY meaningful tests are appreciated.** Do NOT create trivial or fake tests that:
293+ - Just check if something can be instantiated (e.g., ` assert_eq!(schema.fields().len(), 5) ` )
294+ - Print logging statements and then ` assert!(true, "...") ` with no real validation
295+ - Don't actually test functionality or integration behavior
296+ - Artificially inflate test count without proving anything works
297+ - Claim to test "integration" but don't involve any real integration
298+
299+ ** Test Logging Rule: Silent on Success, Verbose on Failure**
300+ - Tests should NOT output logging when everything passes (clean test output)
301+ - Only add logging if a test is FAILING or DEBUGGING to help diagnose the issue
302+ - Tests that pass should be silent - no ` log::info!() ` calls for successful assertions
303+ - This keeps test output clean and prevents noise
304+
305+ ** Test Variable Typing: Always Use Explicit Types**
306+ - All variables in tests MUST have explicit type annotations
307+ - WRONG: ` let expr = col("country").eq(lit("USA")); `
308+ - RIGHT: ` let expr: Expr = col("country").eq(lit("USA")); `
309+ - This makes test code self-documenting and catches type mismatches early
310+ - Type annotations clarify what data flows through the test
311+
312+ Examples of nonsense tests to NEVER create:
313+ - ` test_pushdown_integration_summary() ` - Just prints documentation, asserts true
314+ - Tests that only log messages without any assertions or validation
315+ - Tests that check boilerplate code exists but don't test actual behavior
316+
317+ Every test must prove something meaningful about the system works correctly.
318+
195319### Why Unit Tests Are Mandatory
196320
197321Unit tests are ** non-negotiable** in this project for critical business reasons:
0 commit comments