Skip to content

Fix diskann compilation without default-features and add CI tests.#722

Open
hildebrandmw wants to merge 2 commits intomainfrom
mhildebr/fix-no-default-features
Open

Fix diskann compilation without default-features and add CI tests.#722
hildebrandmw wants to merge 2 commits intomainfrom
mhildebr/fix-no-default-features

Conversation

@hildebrandmw
Copy link
Contributor

Compilation of diskann without the tracing feature is currently broken (see #720). This fixes the issue by:

  1. Uniformly using the tracked_* macros in diskann.
  2. Redirecting the tracked_* macros to a fancy no-op when the tracing feature is disabled.

This fancy no-op uses format_args! to still validate the format of the string and to suppress "unused variable" warnings when the feature is disable. Compiler explorer suggests that with optimizations enabled, the construction of fmt::Arguments is removed, meaning this should have no runtime cost in optimized builds.

Additionally, clippy tests are added for individual crates with features. Feature unification across the workspace results in issues like this not being caught when running CI with the --workspace flag. I tried to be selective with these tests to minimize CI overhead.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes compilation of the diskann crate when the tracing feature is disabled and adds comprehensive CI testing to prevent similar issues. The fix addresses issue #720 by providing conditional implementations of tracing macros that compile to no-ops when the feature is disabled.

Changes:

  • Added feature-gated implementations of tracked_* macros with a used! helper that validates format strings without runtime overhead
  • Replaced direct tracing macro calls (trace!, debug!) with tracked_* variants in diskann/src/graph/index.rs
  • Added CI job to test individual crates with --no-default-features to catch feature unification issues
  • Improved CI reproducibility by adding --locked flag to all cargo commands

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
diskann/src/tracing.rs Added conditional implementations of tracking macros for both with/without tracing feature, introduced used! helper macro for no-op validation
diskann/src/graph/index.rs Removed direct tracing imports and replaced trace! and debug! calls with tracked_trace! and tracked_debug!
.github/workflows/ci.yml Added clippy-no-default-features job for testing crates individually, added --locked flag throughout for reproducibility

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.01%. Comparing base (48b9af8) to head (ac12d36).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #722      +/-   ##
==========================================
- Coverage   89.01%   89.01%   -0.01%     
==========================================
  Files         428      428              
  Lines       78151    78151              
==========================================
- Hits        69566    69563       -3     
- Misses       8585     8588       +3     
Files with missing lines Coverage Δ
diskann/src/graph/index.rs 96.46% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants