Skip to content

test(wseg-build): roaring fixture generator for the Zig reader goldens#9

Merged
igorls merged 2 commits into
mainfrom
feat/aa-roaring-fixtures
Jun 6, 2026
Merged

test(wseg-build): roaring fixture generator for the Zig reader goldens#9
igorls merged 2 commits into
mainfrom
feat/aa-roaring-fixtures

Conversation

@igorls
Copy link
Copy Markdown
Member

@igorls igorls commented Jun 6, 2026

Gated dev-only test (RUN_ROARING_FIXTURES) that regenerates the hybrid-posting fixtures the wormdb-domain-atomicassets ROARING deep-walk is golden-tested against (array-container + bitmap-container cases). Only the bytes ship (committed in the domain repo, companion PR igorls/wormdb-domain-atomicassets#2); rerun after any change to the roaring serialization (e.g. a roaring crate bump) to keep the Rust-writer ↔ Zig-reader byte contract pinned. No-op in CI.

…en tests

Adds a gated dev-only test (RUN_ROARING_FIXTURES) that regenerates the hybrid-posting fixtures
the wormdb-domain-atomicassets roaring deep-walk is golden-tested against
(src/testdata/roaring_{sparse,dense}.bin — array-container + bitmap-container cases). Only the
bytes ship (committed in the domain repo); rerun this after any change to the roaring serialization
(e.g. a `roaring` crate bump) so the Rust writer ↔ Zig reader byte contract stays pinned.
Copilot AI review requested due to automatic review settings June 6, 2026 22:25
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a test module to regenerate hybrid-posting fixtures for testing. The feedback highlights that using a hardcoded absolute Windows path as a fallback is not portable and recommends using relative paths and std::path::Path to ensure cross-platform compatibility.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread crates/wseg-build/src/aa_binfmt.rs Outdated
Comment on lines +633 to +651
let dir = std::env::var("ROARING_FIXTURE_DIR")
.unwrap_or_else(|_| "P:/wormdb-domain-atomicassets/src/testdata".to_string());
std::fs::create_dir_all(&dir).unwrap();
let base = 1_099_511_627_776u64; // 2^40
// (A) sparse → array containers, multiple high-32 keys: base + i*9_000_000, i in 0..600.
let mut a: Vec<u64> = (0..600u64).map(|i| base + i * 9_000_000).collect();
std::fs::write(
format!("{dir}/roaring_sparse.bin"),
encode_posting_hybrid(&mut a),
)
.unwrap();
// (B) dense → a bitmap container (>4096 ids in one 16-bit block): base + i, i in 0..6000.
let mut b: Vec<u64> = (0..6000u64).map(|i| base + i).collect();
std::fs::write(
format!("{dir}/roaring_dense.bin"),
encode_posting_hybrid(&mut b),
)
.unwrap();
eprintln!("wrote roaring fixtures to {dir}");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using a hardcoded absolute Windows path (P:/...) as a fallback default is not portable and will fail or create unexpected directories on non-Windows systems or systems without a P: drive. Additionally, using manual string formatting with / for path construction is not cross-platform friendly.

Consider using a relative path (e.g., assuming a sibling directory structure) and std::path::Path to build paths in a cross-platform manner.

        let dir_env = std::env::var("ROARING_FIXTURE_DIR")
            .unwrap_or_else(|_| "../wormdb-domain-atomicassets/src/testdata".to_string());
        let dir = std::path::Path::new(&dir_env);
        std::fs::create_dir_all(dir).unwrap();
        let base = 1_099_511_627_776u64; // 2^40
                                         // (A) sparse → array containers, multiple high-32 keys: base + i*9_000_000, i in 0..600.
        let mut a: Vec<u64> = (0..600u64).map(|i| base + i * 9_000_000).collect();
        std::fs::write(
            dir.join("roaring_sparse.bin"),
            encode_posting_hybrid(&mut a),
        )
        .unwrap();
        // (B) dense → a bitmap container (>4096 ids in one 16-bit block): base + i, i in 0..6000.
        let mut b: Vec<u64> = (0..6000u64).map(|i| base + i).collect();
        std::fs::write(
            dir.join("roaring_dense.bin"),
            encode_posting_hybrid(&mut b),
        )
        .unwrap();
        eprintln!("wrote roaring fixtures to {}", dir.display());

Copy link
Copy Markdown

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

Adds a dev-only, opt-in test helper in wseg-build to regenerate Roaring-encoded hybrid posting fixtures used as cross-repo goldens for the Zig reader, keeping the Rust-writer ↔ Zig-reader byte contract pinned when Roaring serialization changes.

Changes:

  • Introduces a write_roaring_fixtures test gated by RUN_ROARING_FIXTURES.
  • Generates both sparse (array-container heavy) and dense (bitmap-container) posting sets and writes them as roaring_*.bin fixture files to a configured directory.

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

Comment thread crates/wseg-build/src/aa_binfmt.rs Outdated
Comment on lines +633 to +651
let dir = std::env::var("ROARING_FIXTURE_DIR")
.unwrap_or_else(|_| "P:/wormdb-domain-atomicassets/src/testdata".to_string());
std::fs::create_dir_all(&dir).unwrap();
let base = 1_099_511_627_776u64; // 2^40
// (A) sparse → array containers, multiple high-32 keys: base + i*9_000_000, i in 0..600.
let mut a: Vec<u64> = (0..600u64).map(|i| base + i * 9_000_000).collect();
std::fs::write(
format!("{dir}/roaring_sparse.bin"),
encode_posting_hybrid(&mut a),
)
.unwrap();
// (B) dense → a bitmap container (>4096 ids in one 16-bit block): base + i, i in 0..6000.
let mut b: Vec<u64> = (0..6000u64).map(|i| base + i).collect();
std::fs::write(
format!("{dir}/roaring_dense.bin"),
encode_posting_hybrid(&mut b),
)
.unwrap();
eprintln!("wrote roaring fixtures to {dir}");
…coded path)

The roaring-fixture dev tool defaulted ROARING_FIXTURE_DIR to a hardcoded
`P:/...` Windows path (Gemini + Copilot), which fails / creates stray dirs on
other hosts. Require the env var instead (.expect with a usage hint) — the test
is already gated behind RUN_ROARING_FIXTURES so it stays a no-op in CI.
@igorls
Copy link
Copy Markdown
Member Author

igorls commented Jun 6, 2026

Addressed (commit 7133141):

[Gemini medium + Copilot] hardcoded P:/ fallback (aa_binfmt.rs:651) — removed the host-specific default. ROARING_FIXTURE_DIR is now required when running (.expect with a usage hint); the test is already gated behind RUN_ROARING_FIXTURES, so it stays a no-op in CI and no longer risks creating stray dirs on other hosts. cargo fmt --all --check clean.

@igorls igorls merged commit 3feb99e into main Jun 6, 2026
1 check passed
@igorls igorls deleted the feat/aa-roaring-fixtures branch June 6, 2026 23:32
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