WIP: Explicit u128→u64 truncation cast in duration measurement#1497
WIP: Explicit u128→u64 truncation cast in duration measurement#1497EffortlessSteven wants to merge 10 commits intomainfrom
Conversation
Work item: work-a140ddf6 Fixes clippy::unnecessary_wraps on cmd_doctor() which returns Result<i32> but never produces Err. The function always returns Ok(0) or Ok(1).
Add detailed docstring explaining what validate_config_rules checks: - Duplicate rule IDs - Empty pattern lists - Invalid regex patterns - Invalid multiline_window values - Unknown rule dependencies - Invalid path globs
Work item: work-3d8d9b32
- Remove unused doc comments inside proptest! blocks (clippy warning) - Fix formatting in property_test_string_syntax_invariant.rs - Add missing insta dependency to diffguard-analytics dev-dependencies These are pre-existing issues on the branch that block CI.
…duration_overflow test
Work item: work-976e4427 Decision: Use #[allow(clippy::cast_truncation)] instead of u64::try_from().expect() to align with rust.no_unwrap rule
Add explicit clippy::cast_possible_truncation allow attribute with explanatory comment for the u128→u64 truncation at main.rs:1934. The truncation is safe because u128 millis represents ~584M years; a CLI command cannot approach this limit. Fixes: GitHub issue #297 ADR: ADR-2026-0426-001
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 6 minutes and 46 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (27)
📒 Files selected for processing (33)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| #[test] | ||
| fn language_from_str_yaml_toml_json_aliases() { | ||
| // YAML has 'yml' alias | ||
| assert_eq!("yml".parse::<Language>().unwrap(), Language::Yaml); |
| // YAML has 'yml' alias | ||
| assert_eq!("yml".parse::<Language>().unwrap(), Language::Yaml); | ||
| // JSON has 'jsonc' and 'json5' aliases (JSON with comments) | ||
| assert_eq!("jsonc".parse::<Language>().unwrap(), Language::Json); |
| assert_eq!("yml".parse::<Language>().unwrap(), Language::Yaml); | ||
| // JSON has 'jsonc' and 'json5' aliases (JSON with comments) | ||
| assert_eq!("jsonc".parse::<Language>().unwrap(), Language::Json); | ||
| assert_eq!("json5".parse::<Language>().unwrap(), Language::Json); |
| "Yaml", | ||
| "Yml", | ||
| ]) { | ||
| let lang: Language = alias.parse().unwrap(); |
| "Jsonc", | ||
| "Json5", | ||
| ]) { | ||
| let lang: Language = alias.parse().unwrap(); |
|
|
||
| #[test] | ||
| fn toml_parsed_returns_cstyle() { | ||
| let lang: Language = "toml".parse().unwrap(); |
Green Test Builder Findings — BUILT gateRan the green test suite for work-976e4427 against the Option A implementation ( Edge Cases Covered
All 3 Option A tests passed. Pre-Commit Checks
Known Test ConflictThe test file What Was Not Covered (Known Gaps)
Git State NoteThe working directory was at a different commit than expected when the agent started. Required |
The detect_language() function returns Option<&'static str> which should not be silently discarded. Adding #[must_use] ensures the compiler warns if callers discard the return value.
Refactor Review — HARDENED gateReviewed the implementation of issue #297 against the spec and codebase. No refactoring was necessary. What I checked The fix at What I considered but left unchanged I evaluated extracting the duration cast into a helper function like What needs attention There is a cross-work-item test conflict: Additionally, there is a second i64→u64 cast at line 2618: Summary No refactoring needed. Implementation is sound. The two failing tests in |
…010cb68 The duration_overflow_work_3010cb68.rs test file was created for work-3010cb68 which implemented Option B (saturation) for the u128→u64 truncation fix. However, work-3010cb68 was never merged, and work-976e4427 (this PR) implements Option A (#[allow] attribute) per ADR-2026-0426-001. The test file was testing for Option B patterns which do not exist in the Option A implementation, causing CI failures. Since work-3010cb68 was never merged, this test file is now stale and causing confusion. This removal is consistent with prior attempts (commits 20cfe17, d249b2f) to remove this file from other branches where it was causing CI failures.
2 participants
Closes #297
Summary
Add explicit #[allow(clippy::cast_truncation)] with explanatory comment for the u128→u64 cast at main.rs:1914. This silences the Clippy cast_truncation lint while documenting why the truncation is safe (~584M years uptime required to overflow).
ADR
Specs
What Changed
Test Results (so far)
Implementation was verified at BUILT gate. Full test results to be confirmed at subsequent gates.
Friction Encountered
Notes