Update instructions for agents by harsha-simhadri · Pull Request #1014 · microsoft/DiskANN

harsha-simhadri · 2026-05-05T00:56:29Z

No description provided.

codecov-commenter · 2026-05-05T01:11:05Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.47%. Comparing base (45428af) to head (bfc12be).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1014      +/-   ##
==========================================
- Coverage   89.49%   89.47%   -0.02%     
==========================================
  Files         448      461      +13     
  Lines       84118    85559    +1441     
==========================================
+ Hits        75282    76558    +1276     
- Misses       8836     9001     +165

Flag	Coverage Δ
miri	`89.47% <ø> (-0.02%)`	⬇️
unittests	`89.32% <ø> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 69 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

harsha-simhadri · 2026-05-07T15:42:58Z

+
+---
+
+## Quick Reference


we can probably remove since agents would not how to work with Rust workspace

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Updates and consolidates agent/onboarding and review guidance across the repository, including moving GitHub Copilot-specific review instructions into .github/copilot-instructions.md and splitting agent guidance into crate-local AGENTS.md files.

Changes:

Fixes/updates documentation references for the test baseline caching system in diskann/README.md.
Introduces new onboarding/guidance documents: root AGENTS.md, plus crate-local diskann/AGENTS.md and diskann-wide/AGENTS.md.
Replaces .github/instructions.md with .github/copilot-instructions.md and removes the legacy agents.md.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
diskann/README.md	Updates developer docs for test baselines and comparison helpers.
diskann/AGENTS.md	Adds crate-local agent notes for baseline caching usage.
diskann-wide/AGENTS.md	Adds crate-local agent notes for cross-platform/SIMD validation.
AGENTS.md	Adds a consolidated, repo-wide agent onboarding guide and boundaries.
agents.md	Removes legacy agent onboarding guide (superseded by `AGENTS.md` + crate-local docs).
.github/instructions.md	Removes old GitHub review instructions file.
.github/copilot-instructions.md	Adds Copilot-specific code review instruction set.

Comments suppressed due to low confidence (2)

diskann/README.md:13

The baseline cache directory is documented as diskann/tests/generated, but the crate actually writes to diskann/test/generated (see diskann/src/test/cache.rs uses CARGO_MANIFEST_DIR/test/generated). Update this path here (and any later examples) so developers don’t regenerate into a non-existent tests/ directory.

Developers are strongly encouraged to consider the [caching infrastructure](src/test/cache.rs)
when writing index tests to provide an early warning of algorithmic changes.

This infrastructure serializes test results into a file in `diskann/tests/generated`
that serves as the baseline in the normal test flow. Any difference between the baseline

diskann/README.md:48

This section references diskann::test::cmp::*, but the test module is #[cfg(test)] mod test; and its helpers are pub(crate) (internal-only). To avoid implying this is a public API (and to match in-crate usage), the README should prefer crate::test::cmp::VerboseEq / crate::test::cmp::verbose_eq in examples/instructions.

When comparing baselines, developers should use the `diskann::test::cmp::VerboseEq`
which provides more diagnostics regarding the source of structural inequality than the
standard libraries `PartialEq` trait. Additional utilities include

* `diskann::test::cmp::verbose_eq!`: A macro for automatically implementing `VerboseEq`.
  This macro can be used until a proper `derive` macro is implemented:
  ```rust
  use diskann::test::cmp::verbose_eq;

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

+### Never
+
+- Modify files in `diskann/tests/generated/` by hand — these are auto-generated baselines. Regenerate with `DISKANN_TEST=overwrite`.
+- Modify `rust-toolchain.toml`, `.github/workflows/`, or `.codecov.yml` without explicit approval.
+- Use the global Rayon thread pool — use `RayonThreadPool`/`RayonThreadPoolRef` (enforced by `clippy.toml` disallowed methods).
+- Use `rand::thread_rng` — use the project's `random.rs` utilities instead (enforced by `clippy.toml`).
+- Use `vfs::PhysicalFS::new` or `VirtualStorageProvider::new_physical()` in tests — use `VirtualStorageProvider::new_overlay()`.



 Before checking in new test results, it's a good idea to completely delete `diskann/tests/generated`
 to ensure that unused baselines get removed from the repository.

-The API for registering and retrieving test results is in `diskann/src/tests/cache`
+The API for registering and retrieving test results is in [`diskann/src/test/cache.rs`](src/test/cache.rs)
 and consists of:


hildebrandmw

A few small nits, but otherwise looks good.

hildebrandmw · 2026-05-11T18:01:39Z

+
+- Do not introduce `panic!` paths for recoverable errors — propagate with `Result` instead.
+- Keep error types small. Avoid large enums/structs that blow up the stack; look for ways to reduce field sizes (e.g., compute derivable fields, use enums instead of `&'static str`).
+- Prefer `ANNError::new(ANNErrorKind::…, e)` over the old `log_*`-style constructors, which force eager string formatting and double-log errors.


Nit: There is not double-log from these APIs (the name is a misnomer). The biggest issue is eager string formatting.

hildebrandmw · 2026-05-11T18:19:13Z

+- Doc comments and README examples must match actual API signatures and serialized shapes. 
+- Stale examples that fail to compile or deserialize are treated as bugs.
+- Do not leave dead references to APIs that no longer exist.
+- When changing a function signature or removing a parameter, update all doc comments that mention the old signature.


More suggestions on documentation:

Doc and inline comments should describe what the code does. Do not describe behavior by contrasting it with other code, except when referencing documented external behavior.

Avoid comments that simply restate what is already clear from function signatures or where clauses.

Do not list functions or types in module docs that rustdoc already documents.

Module-level docs should describe the purpose and structure of the module, not its contents.

hildebrandmw · 2026-05-11T18:21:13Z

+
+## Rayon and Parallelism
+
+- Never use the global Rayon thread pool. Always execute parallel work within the provided `RayonThreadPool` or `RayonThreadPoolRef`.


This guidance is more for diskann-providers/diskann-disk than lower level crates. Lower level crates like diskann-quantization should use the dynamically scoped thread pool, but advertise this using the Parallelism enum.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

magdalendobson · 2026-05-11T19:31:45Z

@@ -0,0 +1,182 @@
+# DiskANN Repository - Agent Onboarding Guide
+
+**Last Updated**: 2026-05-04 (based on v0.50.1, Rust 1.92)


It looks like a lot of previous commentary on testing is removed and not reproduced anywhere else. Is this purposeful? It seems like useful information that shouldn't be lost.

Same comment on code coverage. Should we add something on both topics to the "Always" list?

magdalendobson · 2026-05-11T19:36:47Z

+- `diskann-label-filter/` - Inverted index for filtered search
+- `diskann-garnet/` - Garnet (Redis-compatible) Provider and FFI endpoints for vector sets
+
+**Tier 4: Infrastructure & Tools**


"Infrastructure" is a bit of a vague name here, it kind of seems like every tier contains something that could be called infrastructure. Maybe "Benchmarks and Tools" would be better?

magdalendobson · 2026-05-11T19:39:11Z

+
+## Error Handling
+
+There are three regimes of error handling and the strategy to use depends on the regime.


It sounds like these categories may conform neatly to the tiers discussed above, although maybe there are exceptions. If so it could be good to state that explicitly.

magdalendobson · 2026-05-11T19:39:53Z

+    Ok(())
+}
+
+// ❌ Bad — eager string formatting, double-logs on creation


See comment from Mark about double-logging

magdalendobson · 2026-05-11T19:42:53Z

@@ -0,0 +1,64 @@
+When performing a code review, check that:


Can copilot instructions point to the existing AGENTS.md files where applicable? It looks like a lot of instructions are manually duplicated here, so we risk them getting out of sync.

+1.
Do we really need .github/copilot-instructions.md? if so, how is it semantically different from /AGENTS.md? Why can't we keep all agent-related instructions in /AGENTS.md?

magdalendobson · 2026-05-11T19:45:01Z

+
+## Error Handling
+
+- Do not introduce `panic!` paths for recoverable errors — propagate with `Result` instead.


This probably deserves a mention/move to the top-level AGENTS.md

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

arrayka · 2026-05-12T20:31:49Z

@@ -0,0 +1,64 @@
+When performing a code review, check that:


+1.
Do we really need .github/copilot-instructions.md? if so, how is it semantically different from /AGENTS.md? Why can't we keep all agent-related instructions in /AGENTS.md?

arrayka · 2026-05-12T20:47:53Z

@@ -0,0 +1,182 @@
+# DiskANN Repository - Agent Onboarding Guide


I would explicitly follow next structure in this document:

How code is organized (Crate Organization)

How to contribute (aka How we write code) - and move all stuff from copilot-instructions to this section.

How to validate code before submitting a PR + briefly mention that CI will perform extensive validation in .github/workflows/ci.yml.

harsha-simhadri added 2 commits May 4, 2026 12:54

update readme and review instructions

178e0b0

update agents.md

eae10c4

harsha-simhadri commented May 7, 2026

View reviewed changes

harsha-simhadri added 3 commits May 7, 2026 09:04

updated readme

1a9a592

update instructions.ms

89bfe12

moved agents.md to AGENTS.md

4399598

harsha-simhadri changed the title ~~Update README and instructions for agents~~ Update instructions for agents May 9, 2026

harsha-simhadri and others added 3 commits May 10, 2026 16:12

Revert README.md changes

8863e15

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

shorten and split agents.md

981bfce

update review instructions

ba47b16

harsha-simhadri marked this pull request as ready for review May 11, 2026 00:04

harsha-simhadri requested review from a team and Copilot May 11, 2026 00:04

Copilot started reviewing on behalf of harsha-simhadri May 11, 2026 00:04 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

metajack approved these changes May 11, 2026

View reviewed changes

hildebrandmw reviewed May 11, 2026

View reviewed changes

harsha-simhadri and others added 2 commits May 11, 2026 12:07

Potential fix for pull request finding

1019fa9

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

4013d64

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

magdalendobson reviewed May 11, 2026

View reviewed changes

Apply suggestions from code review

bfc12be

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

arrayka reviewed May 12, 2026

View reviewed changes


		## Rayon and Parallelism

		- Never use the global Rayon thread pool. Always execute parallel work within the provided `RayonThreadPool` or `RayonThreadPoolRef`.

		@@ -0,0 +1,182 @@
		# DiskANN Repository - Agent Onboarding Guide

		Last Updated: 2026-05-04 (based on v0.50.1, Rust 1.92)


		## Error Handling

		There are three regimes of error handling and the strategy to use depends on the regime.


		## Error Handling

		- Do not introduce `panic!` paths for recoverable errors — propagate with `Result` instead.

Conversation

harsha-simhadri commented May 5, 2026

Uh oh!

codecov-commenter commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hildebrandmw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov-commenter commented May 5, 2026 •

edited

Loading