chore: post-1.0.0-rc.1 cleanup (non-Node findings) - #253
Merged
Conversation
compile_time_validation.rs showed two macro-error diagnostics in the editor -- "type `User` must `#[derive(Table)]` with `#[hyperdb(register)]`" and "table \"users\" is not registered" -- on a struct that plainly carries `#[derive(Table)] #[hyperdb(table = "users", register)]`. cargo check passes both with and without the compile-time feature, so these were editor-only. Registration is a process-global side effect of expanding derive(Table). Under cargo that is safe: rustc expands a crate's macros in one host process, and struct-level derives expand before function-body macros. rust-analyzer's proc-macro-srv is long-lived and expands lazily, out of order, and from cache, so a query_as! can be re-expanded in a process where no derive ever ran. That is why only two of the six macro call sites in the file were flagged -- the others still had valid cached expansions. Validation now distinguishes "this type is not registered" from "no derive has run here yet" by checking whether the registry is entirely empty, and skips rather than reporting an error it has no basis for. Genuine diagnostics are untouched: once anything is registered a miss is still a real miss, verified by injecting both failure modes into the example -- an unregistered struct still errors with StructNotRegistered, and a nonexistent column still errors with the UnknownColumn diagnostic. Also corrects the rustdoc, which asserted that within-file ordering "is not a concern". True for cargo, and precisely the assumption that broke here. One test needed adjusting rather than the fix being wrong. struct_not_registered_error asserted the diagnostic without registering anything first, so it only passed because some other test in the shared process happened to register earlier -- order is nondeterministic under parallel test execution. It now registers deliberately, which is also a more honest statement of what it tests. Verified stable over 10 consecutive runs. Deliberately not unit-testing the empty-registry skip: every test in that binary shares one process-global registry and they run in parallel, so any such test races between the emptiness check and the call under test. A first attempt at one flaked on run 3 of 5. Gates: fmt, clippy -D warnings (workspace and the out-of-workspace crate), doc, and cargo +1.88 all exit 0; compile-check 13 passed, derive tests pass; the example builds and runs; and rust-analyzer now reports no diagnostics on the file.
Captures the 17 findings left open when the RC shipped, from the three adversarial reviews of PR tableau#250 plus items discovered while cutting the release. Every item was re-verified against main at 033b2da rather than carried over from the review text, so none are stale. Sequenced with the non-Node work first (Tasks 1-9: Rust core, CI, docs) and the hyperdb-api-node findings deferred to Tasks 10-13, so the N-API surface changes land as one reviewable group. Task 1 is the only item with real correctness weight: the grpc client discards per-batch Arrow decode errors, so a corrupt batch yields a silently partial label map. It needs a decision on propagate-vs-log before implementation, so the task states both options rather than presupposing one. Records four things deliberately out of scope with the reasoning: macOS IPC (broken at the pre-migration baseline, needs its own investigation), the qs advisory (patched upstream but blocked by a registry release-age cutoff), the float-saturation behavior (disclosed, not a regression), and the markdown lint backlog (an automated pass corrupted 176 fences, so a retry must track fence state).
…al map get_table_labels and get_column_labels iterated record batches with `if let Ok(batch) = batch_result`, discarding decode failures. A corrupt batch mid-stream therefore produced a map covering only the batches that happened to decode, which a caller cannot distinguish from "this table defines no labels" -- the worst failure mode for metadata used to render UI. The same `if let` chain swallowed schema mismatches through its `downcast_ref` tuple, skipping the entire batch when a column was not TEXT. The `#[expect(clippy::manual_flatten)]` waivers that documented this as deliberate were removed during the edition-2024 let-chain sweep, which took the recorded intent with them. Reviewing it fresh, silence is not defensible here. Both now propagate. Safe to change: the only callers in the tree are in hyperdb-api-salesforce's example, which already uses unwrap_or_default(), and neither method is surfaced through hyperdb-api. Also fixes a latent panic the old code never guarded. RecordBatch::column(1) panics out of bounds, so a batch projecting one column would have aborted rather than errored; there is now a column-count check. The two functions had byte-identical parsing bodies, so the logic is extracted into parse_label_pairs and the bug is fixed once rather than twice, dropping about 50 lines of duplication. Parsing behavior is unchanged -- JSON displayName extraction, verbatim passthrough for plain descriptions, and NULL-row skipping are preserved, and now have unit tests that build real Arrow IPC streams: a truncated stream and a non-TEXT column must both be errors rather than partial maps, which is exactly what the old code returned. Gates: fmt and clippy -D warnings exit 0; cargo test --workspace is 1573 passed / 0 failed, up 5 from the new tests.
The "HYPERD_PATH is not set" error told users to run `cargo run -p hyperd-bootstrap -- download`. That package has not existed since the rename to hyperdb-bootstrap, so the suggestion failed outright: error: package(s) `hyperd-bootstrap` not found in workspace This is the first thing a new user sees when HYPERD_PATH is unset, which makes it the highest annoyance-per-minute defect in the post-RC list. The bootstrap binary's own "no hyperd installed" error had the same problem, suggesting `hyperd-bootstrap download` when the binary is hyperdb-bootstrap. Verified both directions rather than assuming: the old invocation errors as above, and the new one prints the CLI help listing the `download` subcommand. Also corrects the stale name in doc comments across both crates, a root Cargo.toml section comment, and one case that was a wrong *path* rather than just a name -- release.rs pointed at `hyperd-bootstrap/hyperd-version.toml` where the directory is `hyperdb-bootstrap/`. Deliberately left alone: the two User-Agent strings in scrape.rs. They are network-visible identifiers rather than documentation, and that path already carries Akamai bot-protection caveats, so changing them for cosmetic consistency is not worth any behavioral risk. Gates: fmt, clippy -D warnings, and doc all exit 0; hyperdb-api plus hyperdb-bootstrap tests are 595 passed / 0 failed.
The edition-2024 sweep flattened 127 collapsible_if sites and reported them
all fixed. That claim was Linux-scoped: cfg-gated code is stripped before
lints run and the clippy job only ran ubuntu-latest, so every
#[cfg(windows)] block went unlinted. One survivor sat in process.rs's Named
Pipe branch, where an is_some() guard wrapped an if-let on the same Option --
exactly the shape the sweep removed elsewhere. A Windows contributor running
clippy by hand would have hit a hard failure.
Adds windows-latest to the clippy job as a matrix. That leg is the one worth
paying for: Windows carries a whole distinct Named Pipe transport path.
macOS is deliberately left out, since it shares cfg(unix) with the Linux leg,
so only #[cfg(target_os = "macos")] blocks remain unlinted -- a much smaller
surface. The job comment records that reasoning, replacing the old note that
claimed a single runner was enough.
The Windows leg mirrors the test job: choco install protoc, per-OS cache keys,
and the same ${{ github.workspace }}/.hyperd/current form for HYPERD_PATH,
which that job already proves works there. hyperd is needed because
--all-features starts an embedded instance in the proc-macro host.
Verification of the code fix is CI's, not local: cross-compiling to
x86_64-pc-windows-msvc fails here because ring's C build cannot find assert.h
for that target, which is a cross-compile environment limit rather than
anything about the change. Host fmt and clippy exit 0, and the edit is a
mechanical removal of a redundant guard.
The root Cargo.toml promoted missing_errors_doc and missing_panics_doc to deny, but hyperdb-compile-check's copy stayed at warn. That crate declares its own [workspace] so it can build standalone, which means it cannot inherit the workspace lint tables and instead duplicates them -- and the commit that promoted the levels updated only the root. Nothing caught the drift because cargo clippy --workspace skips this crate entirely, so the "measured zero violations" evidence behind the promotion never covered it. Checked explicitly now: the crate satisfies both at deny with no changes needed, so the promotion is a no-op for its code and purely closes the inconsistency. Also documents why the duplication exists and how to lint the crate, since a forced-duplicate table with no enforcement is exactly what drifted. The note records the specific drift so the next person understands the hazard rather than assuming the copy is decorative. Gates: clippy -D warnings and cargo +1.88 check --locked both exit 0 on the crate's own manifest.
The test comment credited `split_at_checked` plus `slice::get` with removing the 32-bit `4 + len` overflow, but the shipped code uses `split_first_chunk::<4>`. `split_at_checked` was the earlier attempt that commit d7157c3 describes discarding, so the comment sent the next reader looking for a call that is not there. Also states the reason more precisely: the fix is not that those calls are overflow-checked, it is that they remove the arithmetic entirely, leaving nothing to overflow.
Three issues the CI review found in rhel-compatibility.yml, plus one trivial Makefile fix. protoc was fetched over HTTPS and unzipped into /usr/local as root with no integrity check. Version-pinning does not help against a retagged or compromised release, so the archive's sha256 is now verified before it is unpacked. Upstream publishes no checksum file for protobuf releases, so the digest is computed from the asset and pinned in env: with the refresh command recorded next to it. Verified both directions locally: the pinned digest matches the real asset, and a tampered digest fails the check. The job had no concurrency group, unlike every other workflow in the repo. Since it builds a container and the whole workspace, successive pushes to a PR stacked multi-minute runs. Now serialized per ref, cancelling in-progress runs on pull_request only. Its cargo check --workspace could not see hyperdb-compile-check, which declares its own [workspace] -- yet release.yml publishes that crate. The one gate that proves "builds on Red Hat's system toolchain with no rustup" was therefore skipping a crate enterprise consumers can depend on. Now checked explicitly, matching what the msrv job already does. Also adds the long-missing `help` to the Makefile's .PHONY list, and confirms `make help` still works. Verification of the workflow itself is CI's: `make check-rhel` needs a container runtime that is not currently provisioned here. The shell forms were validated directly instead.
Both of its claims were wrong after the 1.88 uplift. It framed edition 2024 as something "a few of our transitive deps (rmcp, rmcp-macros, base64ct, clap_lex)" use. The entire workspace is edition 2024 as of 1.0.0, so the rust-analyzer incompatibility it warns about is now unavoidable rather than incidental. It also told contributors to run `rustup component add rust-analyzer` by hand. rust-toolchain.toml lists rust-analyzer in its components, so rustup provisions it with the toolchain -- commit 091327c claimed to obviate that instruction and the batched docs pass never followed through. The remaining step, pointing the extension at that binary via user settings, is kept because it is still needed and still deliberately uncommitted. Adds a note on the rust-analyzer registry quirk fixed in 22b0a7a, since an editor-only false error on code that cargo accepts is exactly the kind of thing a contributor would otherwise burn time on.
The "Rust vs Node.js — 10M apples-to-apples" table quoted Rust figures at 10M rows, but only the 100M table was published, so not one Rust number in the comparison could be checked against a source. Worse, the 100M table sits directly above it, inviting the reader to assume the comparison came from there. Re-measured the Rust suite at 10M (median of 5 runs, same host and hyperd) and published it, then aligned the comparison rows to those exact medians. All eight Rust figures in the comparison now appear verbatim in the table above it, verified programmatically rather than by eye. Re-measuring corrected a review finding rather than confirming it. The aggregation cell reads ~1 K/s, which the review flagged as wrong on the grounds that the measured value is 199/s. That comparison was itself apples-to-oranges: 199/s is the 100M figure, while at 10M the same workload is 10 rows in 0.007s, or about 1.4 K/s. The cell was right; it was merely unverifiable. Left as-is. Also softens the first takeaway, which led with a x4 number and derived a two-significant-figure "2.4x speedup" from it -- directly contradicting the callout immediately above that warns those rows carry +/-20-61% spread and should be read as order-of-magnitude.
… at all `make test` covered 3 of 8 crates while CI's test job covers 7, so the "1519 passed" figure quoted throughout the 1.88 uplift looked like whole-workspace coverage when it was a subset. AGENTS.md recommends `make test` over `cargo test` precisely so HYPERD_PATH is set, which made the gap more misleading rather than less. It now mirrors CI's invocation, giving 1586. That surfaced a real hole: nothing anywhere ran hyperdb-compile-check's tests. It declares its own [workspace], so `--workspace` cannot see it, and every reference to it in CI is a `cargo check` or a publish step -- the msrv job, the RHEL job, release.yml. Its 13 unit tests, including the registry logic changed in 22b0a7a, had never executed in CI. Added to both `make test` and the CI test job. Also gives the crate the CHANGELOG.md it never had. It is published (release.yml does so explicitly for the same out-of-workspace reason), but was absent from AGENTS.md reminder 8's list of publishable crates, which is why it was missed. That list and the "eight per-crate changelogs" count are now nine, with a note recording why the crate is easy to overlook. Verified: make test exits 0 at 1586 passed / 0 failed, which reconciles as the 1573 from `cargo test --workspace` plus this crate's 13.
Stripping two never-tracked files (.codex/config.toml and a stray verify_release.py) from this branch used `git rm -r --cached .agents .codex`, which was too broad: .agents/skills/*/SKILL.md are legitimately tracked on main, so the rebase deleted them too. Restored verbatim from upstream/main, leaving this branch's net diff for both directories empty. The two never-tracked files remain untracked, which is where they belong -- they are local agent config, not repository content.
…at were mine markdownlint is not a CI gate, so the only feedback is the editor extension -- and an agent working headless gets none. That gap produced defects twice in one session: duplicate changelog headings, untagged code fences, and a bulk auto-fix that corrupted 176 fences across 22 files by mistaking closing fences for opening ones. Adds reminder 3 with the invocation, an explanation of why there are three config files (markdownlint-cli2 does not read .markdownlintignore; the editor extension does), and the three traps that have actually bitten. It also warns that a pre-existing backlog exists, so a nonzero count is not automatically yours -- judge against `git show upstream/main:<path>` rather than assuming, or you fix things that were never broken and miss what you introduced. Fixes the 4 of 10 current findings that this branch introduced, all MD024: each changelog already had a `### Fixed` or `### Added` further down the [Unreleased] section, and I appended a second one at the top. Merged into the existing sections, which also restores Keep a Changelog ordering (Changed before Fixed). Content verified intact after the merge. The remaining 6 are pre-existing and verified as such against upstream/main, including the 708-character KvStore line and the two hyperdb-mcp duplicates, which are byte-identical there. Writing the reminder promptly demonstrated the need for it: the first draft added 3 MD007 violations of its own, since AGENTS.md carries two nested-bullet styles and only the flush-left one lints clean.
`hyperdb-api/src/lib.rs` opts this crate back in to
`#![warn(clippy::must_use_candidate)]` (the lint is `allow` workspace-wide
because it measures public-API ergonomics, which only matters for the
flagship crate). CI promotes it to an error via `-D warnings`.
`HyperProcess::pipe_name` is `#[cfg(windows)]`, and `cfg`-gated code is
stripped before lints run, so the ubuntu-only clippy job never compiled it
and could not flag it. The newly added `clippy (windows-latest)` matrix leg
compiles the Named Pipe transport path for the first time and surfaced it:
error: this method could have a `#[must_use]` attribute
--> hyperdb-api\src\process.rs:968:12
Annotated to match the neighbouring `#[cfg(unix)]` `socket_directory`,
which already pairs the `cfg` attribute with `#[must_use]`.
Audited every Windows-gated region in `hyperdb-api/src` (the only crate
where the lint is `warn`) for further candidates: this was the sole one.
The only other `cfg(windows)`-gated `fn` is `AsyncTransport::connect_named_pipe`,
which cannot fire the lint — it is `pub(crate)` (not exported), `async`, and
returns `Result`, and both `Future` and `Result` are already `#[must_use]`.
Verified on the host: `cargo clippy --workspace --all-targets --all-features
-- -D warnings` and `cargo fmt --all -- --check` both exit 0. The Windows
target cannot be linted locally (`ring`'s C build cannot locate `assert.h`
when cross-compiling to `x86_64-pc-windows-msvc`), so the windows-latest CI
leg is the authority here.
Second finding from the new `clippy (windows-latest)` leg. The first one
(`must_use_candidate` on `HyperProcess::pipe_name`) aborted compilation of
`hyperdb-api`, so every crate downstream of it went unlinted — fixing that
let the job reach `hyperdb-mcp` and report:
error: used underscore-prefixed binding
--> hyperdb-mcp\src\diagnostics.rs:1027:50
= note: `-D clippy::used-underscore-binding` implied by `-D warnings`
`resolve_configured_hyperd` takes the configured HYPERD_PATH in both parsed
and raw-text form. Only the `#[cfg(windows)]` `.exe` fallback consults the
raw text, so off Windows the parameter is genuinely unused and the underscore
kept rustc's `unused_variables` quiet. On Windows the underscore is a lie,
which is exactly what `used_underscore_binding` detects.
Dropping the underscore alone would break the ubuntu and macOS legs with
`unused_variables` instead, so the name is now honest and the off-Windows
discard is explicit — the same idiom already used at `watcher.rs:769` and
`process.rs:998`. `items_after_statements` is `allow` workspace-wide, so the
discard may precede the `HYPERD_EXE` consts.
Also audited every underscore-prefixed binding in all eight crates for the
same cfg-dependent pattern; this was the only one. Separately re-scanned all
88 Windows-gated regions for other enabled-lint triggers: the two
non-inlined `format!` args in `process.rs` sit in the
`#[cfg(not(any(unix, windows)))]` exotic-platform fallback, which no CI
runner compiles, and `unwrap_used` is `restriction`-level and not enabled.
Host gate still clean: `cargo clippy --workspace --all-targets --all-features
-- -D warnings` and `cargo fmt --all -- --check` both exit 0.
Third finding from the new `clippy (windows-latest)` leg, surfaced only
after the previous two stopped aborting the crates ahead of it — this one
is in a test target, which `--all-targets` lints:
error: this `repeat().take()` can be written more concisely
--> hyperdb-mcp\tests\doctor_tests.rs:850:17
= note: `-D clippy::manual-repeat-n` implied by `-D warnings`
The `#[cfg(unix)]` twin of this helper, ten lines above, already used
`std::iter::repeat_n`. Only the `#[cfg(windows)]` copy still had the old
`repeat().take()` form, because no CI runner had ever compiled it — the
exact class of drift the Windows leg was added to catch. Now mirrors its
Unix sibling.
Completes the audit of the Windows-gated surface. Every `#[cfg(windows)]` /
`#[cfg(not(unix))]` region in the workspace has now been reviewed against
the enabled lint groups, across all target kinds rather than just `src/`
(the earlier passes missed test targets, which is how this one survived):
`hyperdb-mcp` tests, `hyperdb-api` tests and benches, and the `src` trees
of `hyperdb-api`, `hyperdb-api-core` and `hyperdb-mcp`. This was the only
remaining `repeat().take()` in the repository. The residual
`#[cfg(not(any(unix, windows)))]` fallbacks are compiled by no CI runner
at all, so their contents stay unlinted by construction.
Host gate clean: clippy and fmt both exit 0.
Fourth finding from the new `clippy (windows-latest)` leg, in the `lib test`
target of `hyperdb-mcp`:
error: unused import: `PathEncoding`
--> hyperdb-mcp\src\daemon\discovery.rs:471:30
= note: `-D unused-imports` implied by `-D warnings`
`PathEncoding` has exactly one use in the file, inside a `#[cfg(unix)]` block
that builds a deliberately non-UTF-8 path from raw bytes via
`OsStringExt::from_vec`. That construction has no Windows analogue, so on
Windows the import resolved to nothing and `unused_imports` fired. Moved into
the block that uses it, alongside the `use std::os::unix::ffi::OsStringExt;`
already scoped there.
Audited the workspace for the same shape — a symbol imported at module scope
but consumed only by `cfg(unix)` / `cfg(not(windows))` code. This was the only
genuine instance. The other candidates surfaced are trait imports used through
method-call syntax (`Read`/`Write` in `sync_stream.rs`, `AsyncRead`/
`AsyncWrite` in `async_stream.rs`, `io::Write` in `daemon_tests.rs`) whose
`impl` blocks and `write!` call sites are ungated, so they stay used on
Windows.
Host gate clean: clippy and fmt both exit 0.
Updates `version`, `build_id`, and all four per-platform sha256s in `hyperd-version.toml`, moving the pin from `0.0.26359` (`r07abb490`). Verification - `make verify-hyperd-pin` — all four platform URLs HTTP 200 at the new pin. - `make download-hyperd` — sha256 verified against the toml on the real install path. - `.hyperd/current/hyperd --version` → `main.0.0.26479.r96880f6a`. - `file .hyperd/current/hyperd` → `Mach-O 64-bit executable arm64`. The Java bundle still carries a **native arm64** binary, so the reason this crate prefers it over the C++ bundle (whose macos-arm64 zip ships an x86_64 `hyperd`) continues to hold. - `make test` — **1586 passed / 0 failed** (50 ignored, 99 result lines) against the new engine, matching the expected baseline across all 8 crates including the out-of-workspace `hyperdb-compile-check`. - `cargo fmt --all -- --check` and `cargo clippy --workspace --all-targets --all-features -- -D warnings` both exit 0. Performance Interleaved A/B (old, new, old, new, …) so thermal drift loads both engines equally; medians of 5 runs per engine at 100M rows on an M3 Max 14-core laptop. The async Arrow insert path more than doubles; nothing else moves: | workload (single connection) | 0.0.26359 | 0.0.26479 | Δ | |---|---:|---:|---:| | async AsyncArrowInserter | 30.35 | 68.90 | +127.0% | | sync Inserter | 25.43 | 25.46 | +0.1% | | sync ChunkSender | 24.52 | 24.86 | +1.4% | | sync query.full_scan | 31.29 | 31.08 | −0.7% | | async query.full_scan | 24.82 | 24.88 | +0.2% | | sync query.filtered | 33.54 | 33.75 | +0.6% | | async query.filtered | 26.57 | 26.73 | +0.6% | M rows/s. The insert gain is not a variance artifact despite that workload's 23–35% run-to-run spread: the two sample ranges are disjoint (old 24.51–31.43, new 45.56–69.95) and the effect reproduces at 10M rows (28.22 → 49.39, +75%, also disjoint). Multi-connection (`× 4`) deltas are withheld per the tracker's methodology — 17–41% spread on this host, and the same workload read +23% at 100M but −20% at 10M in one session, so they carry no signal. The baseline leg was re-measured on the current API rather than taken from the recorded 2026-08-24 row, which was collected at `0.7.x`. It reproduces that row within ~1–5%, which is the evidence that the 0.7.x → 1.0.0-rc.1 API change did not move these numbers and that the delta above is the engine's. Docs - `docs/hyperd-release-benchmarks.md`: one insert row and one query row per engine, both at API `1.0.0-rc.1`. - `docs/BENCHMARK_GUIDE.md`: the macOS provenance now names the new pin, and the single-connection `AsyncArrowInserter` row is restated in both the 100M and 10M tables. The rows that only moved within run-to-run spread are carried forward and labelled as such — re-rolling them would have published a spurious −18% on `query.full_scan × 4`. Because the fastest Rust insert at 10M is now the single-connection async path rather than the `× 4` one, the Rust-vs-Node row flips from "0.9× (Node ahead)" to 1.2×; its Rust figures still appear verbatim in the 10M table above it. That row is flagged as mixing engine versions, since the Node bench was not re-run and Node's `ArrowInserter` shares the `hyperd` ingest path that got faster. - `hyperdb-bootstrap/CHANGELOG.md`: bullet merged into the existing `### Changed` under `## [Unreleased]`. `npx markdownlint-cli2` introduces no new findings in the three touched Markdown files (repo backlog 128 → 126).
Headline performance numbers were duplicated in prose across the repo and were left understated for the async insert path by the 0.0.26479 hyperd bump (AsyncArrowInserter 30.35 -> 68.90 M rows/s, +127%). docs/BENCHMARK_GUIDE.md and docs/hyperd-release-benchmarks.md were updated at the time; these files were not. Also fixes a correctness bug, not just a number: AGENTS.md credited `ArrowInserter` with 30M rows/sec, but the suite measures the async `AsyncArrowInserter`. The sync inserters measured 25.5 M rows/s and did not move with this engine, so the name was wrong and the figure now belongs to a different API than the one named. Every remaining claim names the exact API and the connection count it was measured at. Multi-connection (x 4) figures are dropped from prose rather than restated: they carry a 17-41% run-to-run spread on this hardware and read as deltas of both signs at different scales, so they are order-of-magnitude only and now live solely in the guide, which documents that spread. DEVELOPMENT.md's Rust-vs-C++ table is deliberately left unchanged and marked historical instead. The C++ side has not been re-measured, so refreshing only the Rust column would produce a cross-engine comparison that means nothing.
The update-hyperd-release skill documented hyperd-version.toml as "the whole source of truth", but the pin is duplicated in .github/workflows/npm-build-publish.yml (HYPERD_VERSION, HYPERD_BUILD_ID, and three matrix hyperd-sha256 values, plus a fourth in the commented-out darwin-x64 block). .github/scripts/verify-npm-hyperd-pin.py fails CI on any drift, so the omission cost a red `verify` check on the 0.0.26479 bump, which updated only the toml. Adds a step 5 adjacent to the "edit hyperd-version.toml" step, since the two files must move together: which keys to change and where, that the matrix hashes are the Java-zip sha256s the guard compares directly against the toml's values, that hyperd-slug uses the toml's platform names rather than npm's, and that the commented-out darwin-x64 entry is invisible to the guard but should be kept current so re-enabling those runners does not fail. Records why the guard exists: the pins drifted once and shipped npm packages with engine 0.0.25080 while crates.io shipped 0.0.26359. Applied identically to both byte-identical copies of the skill.
Pin the next release to 1.0.0-rc.2. This branch carries a breaking commit (fix(grpc)! on Arrow label-lookup failures), so without this footer release-please computes 2.0.0-rc.1 the moment the branch merges. 1.0.0 has not shipped yet, so the break belongs inside the rc line rather than driving a major bump. Release-As: 1.0.0-rc.2
StefanSteiner
added a commit
that referenced
this pull request
Sep 6, 2026
…254) ## Motivation `hyperdb-bootstrap` fetched `hyperd` from Tableau's Hyper **Java API zip**, whose filename embeds an opaque `build_id` (e.g. `r07abb490`) that **cannot be derived from the version**. Discovering it meant scraping the public releases page — and that scraper (`src/scrape.rs`, behind `--latest`) has been **broken for three-plus releases without anyone noticing**, because its tests ran against a synthetic fixture rather than the live page. Two independent defects: - the heading regex expects `<h3>VERSION [DATE]</h3>`, but Docusaurus renders `0.0.26479 <!-- -->[September 3 2026]`, which `\s*` cannot span; - the build-id capture hardcodes `(rc[a-z0-9]+)`, while every build id since `0.0.24457` has been `r` + hex. The PyPI `tableauhyperapi` wheels carry the same engine behind a **fully constructible** URL, and **PyPI publishes a sha256 per file**: ```text https://files.pythonhosted.org/packages/py3/t/tableauhyperapi/tableauhyperapi-{version}-py3-none-{wheel_tag}.whl ``` So `--latest` is **deleted, not fixed**: with a constructible URL and published digests there is nothing left for it to do. ## The bytes are unchanged The strongest evidence this migration is faithful — the `hyperd` extracted from the `macosx_13_0_arm64` wheel is **bit-identical to the `hyperd` in the Java zip for the same release**, sha256 `aef5c81970bb4d84d06fb9513d5ffd722526fce779632a0c5f63d87b6450e478`. To be precise about what is and isn't established here: that cross-envelope equality was established upstream of this PR and I did not re-derive it. What I verified locally is that the wheel path produces exactly that binary: ```text $ shasum -a 256 .hyperd/current/hyperd aef5c81970bb4d84d06fb9513d5ffd722526fce779632a0c5f63d87b6450e478 $ stat -f%z .hyperd/current/hyperd 277836448 $ file .hyperd/current/hyperd .hyperd/current/hyperd: Mach-O 64-bit executable arm64 $ .hyperd/current/hyperd --version Hyper version main.0.0.26479.r96880f6a ``` Same build, different envelope. Note the `--version` line corroborates this independently: the wheel's binary self-reports build `r96880f6a`, which is exactly the `build_id` the Java zip carries for `0.0.26479` (see `chore/post-rc-cleanup`'s `8c99d20`). It is the same engine build reached by a different filename. This is *not* a claim that the wheel matches the engine currently on `main` (`0.0.26359`, build `r07abb490`) — that is a different build. The equality is same-release, cross-envelope. Both binaries report `minos 13.0`, so the `macosx_13_0` wheel tag is **not** a raised support floor — no contributor loses support. Wheels are ~3.6–4.5% smaller than the Java zips. Bumping the pin no longer means downloading four ~80 MB archives and hashing them by hand — the digests come off the JSON API. They are **still committed**: a hash in git is an attestation independent of the host serving the bytes. ## Breaking changes Public API removed: | Removed | Replacement | |---|---| | `PinnedRelease::build_id`, `InstalledHyperd::build_id` | `.version` is the only release identifier | | `PinnedRelease::version_tag()` | `.version` | | `VersionSource::ScrapeLatest`, the `scrape` module | none — deleted | | `Error::Http`, `Error::HttpStatus`, `Error::ScrapeFailed` | none — served the scraper only | | CLI `--latest`, `--build-id` | `--version X` alone is now a complete source | | `regex`, `reqwest`, `rustls` deps | none — no in-process HTTP client remains | `url::build_download_url` is now **fallible** (`Result<String, Error>`); a platform with no pinned wheel tag is `Error::MissingWheelTag` rather than a guess. `PinnedRelease::wheel_tag_for(Platform)` is new. The install layout is keyed on the version alone: `<dest>/0.0.26479/` (was `<dest>/0.0.26479.r96880f6a/`), and `current/VERSION` now contains just `0.0.26479`. Nothing in the repo reads that file programmatically. Dropping `reqwest`/`rustls` also **retires the rustls crypto-provider workaround** the CHANGELOG records as a past breaking change (`rustls-no-provider` plus a `OnceLock` installing ring). Verified `aws-lc-rs` is absent workspace-wide and that `hyperdb-bootstrap` no longer reaches `reqwest`, `rustls`, or `regex` at all. ## Versioning: breaking, released as `1.0.0-rc.2` **Decided — no reviewer action needed.** The break is now marked honestly *and* the version is pinned, which are two independent things: - The subject is `feat(bootstrap)!:` and the migration commit carries a `BREAKING CHANGE:` footer enumerating every removed item. The break is real and is recorded as such. - An empty follow-up commit, `chore: release 1.0.0-rc.2`, carries a `Release-As: 1.0.0-rc.2` footer, which pins the next release inside the `1.0.0-rc` line. Without that footer the `!` would compute **2.0.0**: the workspace is already on `1.x`, and `bump-minor-pre-major` in `release-please-config.json` is gated on `version.isPreMajor` (`major < 1`), so it does not apply at `1.0.0-rc.1`. `Release-As:` wins regardless — release-please's `DefaultVersioningStrategy` returns a `CustomVersionUpdate` from the `RELEASE AS` note *before* it reads the breaking-change tally. This is the same mechanism, and the same empty-commit shape, that produced `1.0.0-rc.1` (`7bf2dff`), and it is what [`docs/GITHUB_OPERATIONS.md` → Pre-releases](https://github.com/tableau/hyper-api-rust/blob/main/docs/GITHUB_OPERATIONS.md#pre-releases) documents. Simulated against `release-please@17.11.2` (the version `release-please-action@v5` pins) using its real `parseConventionalCommits` and `DefaultVersioningStrategy`: `1.0.0-rc.1` → **`1.0.0-rc.2`** under a merge commit *and* under a squash merge. Dropping the footer from the same input yields `2.0.0-rc.1`, which is the outcome this pin exists to prevent. > **Do not merge a release PR that says anything other than `1.0.0-rc.2`.** > #253 merges first, and release-please will run on that push and open a > `chore(main): release …` PR computed from #253's commits alone — before this > PR's `Release-As:` footer is on `main`. That PR will show the wrong version. > It updates itself once this PR lands; leave it alone until then. ## Why wheel tags live in the pin file They are deliberately *not* hardcoded in Rust. They are not guaranteed stable across releases (arm64 wheels only exist from `0.0.19484`; a future macOS floor bump would change `macosx_13_0_arm64`), and a wrong tag is a **silent 404** on one platform only. Keeping them as pin data makes any such change a visible pin edit, and `build.rs` now fails the build if any supported platform lacks one. (Empirically the four tags are unchanged from `0.0.19484` through `0.0.26479`.) ## Stronger `verify` `hyperdb-bootstrap verify` now cross-checks every pinned digest against the digest PyPI publishes for that exact wheel filename, on top of HEAD-ing the four URLs — so it validates the **exact pinned bytes** rather than merely that the CDN serves something at that path, and it catches a stale `[wheel_tag]` explicitly: ```text $ cargo run --release -p hyperdb-bootstrap --bin hyperdb-bootstrap -- verify verifying hyperd 0.0.26479... OK macos-arm64 [200] .../tableauhyperapi-0.0.26479-py3-none-macosx_13_0_arm64.whl digest matches PyPI OK macos-x86_64 [200] .../tableauhyperapi-0.0.26479-py3-none-macosx_10_11_x86_64.whl digest matches PyPI OK linux-x86_64 [200] .../tableauhyperapi-0.0.26479-py3-none-manylinux2014_x86_64.whl digest matches PyPI OK windows-x86_64 [200] .../tableauhyperapi-0.0.26479-py3-none-win_amd64.whl digest matches PyPI all platforms reachable with matching digests. ``` ## npm dual pin `npm-build-publish.yml` carries its own independent pin (this is why `0.7.1` once shipped npm with a different engine than crates.io). It is migrated to the wheel URL, `HYPERD_BUILD_ID` is gone, and the matrix gains `hyperd-wheel-tag`. `verify-npm-hyperd-pin.py` keeps its version + digest cross-check and **gains a wheel-tag cross-check**, since the tag is a new drift vector. Empirically confirmed (not assumed) that the matrix `hyperd-sha256` values are digests of the **downloaded archive**, not the extracted binary — the step hashes `hyperd-archive.whl` — so they are the wheel digests and equal the toml's `[sha256]`. I did **not** make the workflow read the toml directly. The matrix must be static YAML, so it would need a `tomllib` call inside the per-platform matrix job, which runs on a Windows runner under git-bash where `python3` may not be on `PATH` — an untestable portability risk for no correctness gain over the guard. **Recommended as a follow-up** alongside converting the matrix to a `setup`-job JSON output. ## Verification Run on macOS arm64 (Apple Silicon): - `cargo build -p hyperdb-bootstrap` and `cargo build --workspace` — clean - `cargo test -p hyperdb-bootstrap` — 26 lib + 4 integration + 1 doc, 0 failed (up from 12 lib + 2 integration; +16 runnable tests) - `cargo test --workspace` — **1584 passed, 0 failed**, against the freshly downloaded engine - `make test` — **1519 passed, 0 failed** - `cargo fmt --all -- --check` and `cargo clippy --workspace --all-targets --all-features -- -D warnings` — clean - `npx markdownlint-cli2` — zero new findings; the 5 in `AGENTS.md` / `DEVELOPMENT.md` are the pre-existing MD040 backlog, identical before and after - `verify-npm-hyperd-pin.py` passes against the new toml, and **fails correctly** when a wheel tag is perturbed - The workflow's `find`-based extraction was replayed against the real wheel entry names: `HYPER_DIR=hyperd-raw/tableauhyperapi/bin/hyper`, `hyperd` found, `LICENSE*`/`NOTICE*` glob matches `dist-info/` `extract.rs` is **unchanged** apart from doc comments — its "skip one optional top-level directory, then require a `lib/hyper` or `bin/hyper` pair" logic already absorbs the `tableauhyperapi` wrapper. New tests pin that against the real wheel entry names, including the Windows `hyperd.exe` + `crashdumper.exe` case. ### Not verified here - **Only macOS arm64 was executed.** The other three platforms rest on CI. Their URLs and digests *are* machine-verified (all four HEAD 200 with matching PyPI digests), and I downloaded the Windows and Linux wheels to confirm their `bin/hyper/` contents (`hyperd.exe` + `crashdumper.exe`; `hyperd`) and digests — but no `hyperd` was run on Linux or Windows. - The engine links only system frameworks (`otool -L`) and `bin/hyper/` contains no shared libraries on any of the three wheels I inspected, so the npm shared-library copy loop finds nothing to copy — as before. --- Repeated here as a trailing footer so the version is pinned on the merge commit too: this repo merges with `merge_commit_message = PR_BODY`, so the body below becomes the merge commit's body. Same value as the footer on `chore: release 1.0.0-rc.2`, so it is redundant rather than conflicting. Release-As: 1.0.0-rc.2
StefanSteiner
added a commit
that referenced
this pull request
Sep 6, 2026
… bench harness (#256) Two independent fixes, one commit each. They share no code and can be reviewed separately. - `6f4e739` — `fix(mcp)`: identify `hyperd` by executable, not thread name - `77834dc` — `fix(bench)`: report decimal MB, not MiB under an `MB` label --- ## 1 — The flaky watchdog test was a real Linux bug, not timing **What was wrong.** `slow_health_watchdog_reaps_hyperd_after_child_timeout` failed intermittently in CI. The cause was not timing. The reaping guard `validate_hyperd_process` confirmed a process's identity with `ps -p <pid> -o comm=`. On Linux that reads `/proc/<pid>/comm` — the **main thread's name**, not the process image — and `hyperd` renames its main thread to `hyperdMain` via `pthread_setname_np` at startup. The guard therefore concluded the PID was not `hyperd`, and `stop_reported_hyperd` refused to signal the very process it exists to reap. **Why it stayed invisible for so long.** Two things hid it: - The guard is normally skipped. By the time the watchdog fires the engine is usually already dead, and an `Err(_) if !process_is_alive(pid)` arm forgives an identity failure on an exited process. The test was passing for the wrong reason — it only failed when the engine was still alive at that moment. - macOS cannot observe the bug at all. There, `ps -o comm=` prints the executable path rather than a thread name, so every local run is green regardless. **Measured on real Linux.** The old guard rejected a genuine, live `hyperd` **200 out of 200 times**. The new `/proc/<pid>/exe` guard rejected it **0 out of 200 times**. **The fix.** Read identity from the kernel's record of the mapped executable (`/proc/<pid>/exe`) on Linux, which no `prctl`/`pthread_setname_np` can rewrite, and keep `ps` on macOS/BSD where it is sound. This is engine-independent: the previous `0.0.26359` pin flakes at the same rate as the current `0.0.26479`, so it is not a regression from the pin bump. All three `slow_health_*` tests shared this fragility and the single fix covers them. A new test, `hyperd_identity_guard_accepts_a_live_engine`, exercises the identity path unconditionally, so it now runs on every build instead of only when the timing happens to expose it. --- ## 2 — `MB/sec` silently meant two different units **What was wrong.** `BenchRecord::mb_per_sec()`, `fmt_mb()` and the `memory_*_mb` helpers divided by 1024² while labelling the result `MB` — so they emitted MiB. The sibling formatters in the same module (`fmt_count`, `fmt_rate`, `fmt_size`) were already decimal, so the harness was internally inconsistent. **Why it stayed invisible.** The two conventions are only 4.86% apart, and both landed in the *same column* of `docs/BENCHMARK_GUIDE.md`: the macOS tables were decimal MB while the Windows tables were raw MiB, under one shared `MB/sec` header. Nothing in the output distinguished them. **The arithmetic that pins it down.** - macOS full scan, 3.218 s → the table reads **745.8**. Decimal MB gives 745.8; MiB would give 711.3. So that row is decimal. - Windows sync `Inserter`, 22.716 s → the table reads **100.8**. That is MiB; decimal would give 105.7. So that row is binary. **The fix.** Make the harness decimal behind a documented `BYTES_PER_MB` constant, rather than relabelling the column to MiB. Relabelling would have been the wrong direction: the macOS figures were freshly re-measured and are genuinely decimal, so calling them MiB would overstate them by 4.86%. Installed RAM stays binary — a 36 GiB machine must read `36.0 GB`, not `38.7` — with a comment saying why it is deliberately the exception. The Windows tables are **not converted.** Their header is relabelled to an explicit `MiB/sec` and footnoted instead, because that run is stale on three other axes as well; multiplying by 1.048576 would dress stale data up as a fresh sample. **Test placement is load-bearing here.** Coverage went into a new `hyperdb-api/tests/bench_common_tests.rs` target. The benches are registered as *examples* with `autobenches = false`, so `cargo test --benches` runs nothing and a `#[test]` inside a bench file would never execute at all. Also fixed in passing: a narrowing `i64 as i32` in `gen_id`, which silently wrapped past 2^31 rows and emitted duplicate and negative IDs — corrupting the very throughput numbers being measured. It is now `i32::try_from(..).expect(..)`. --- ## Deliberately left for follow-ups - The same narrowing `i64 as i32` pattern remains in the per-row generators of `benchmark.rs`, `async_parallel_benchmark.rs` and `benchmark_suite.rs`. Kept out of a units commit on purpose. - `hyperdb-api-salesforce/examples/salesforce_auth_example.rs:425` has the identical MiB-labelled-as-MB defect. - The Windows tables need re-measuring on a Windows host before both table headers can be unified to `MB/sec (10⁶ B/s)`. --- ## Verification No rebase was needed. This branch's base (`chore/post-rc-cleanup`) landed in `main` as #253, and `main`'s only change since is the npm workflow's mirrored `hyperd` pin — a file this branch does not touch. `git log upstream/main..HEAD` is exactly the two commits above. Re-run against `main` at `9dabbef`: | Gate | Result | | --- | --- | | `cargo fmt --all -- --check` | clean, exit 0 | | `cargo clippy --workspace --all-targets --all-features -- -D warnings` | exit 0, 0 diagnostics (483 artifacts, 75 test targets) | | `make test` | **1597 passed, 0 failed**, 50 ignored, exit 0 | | `npx markdownlint-cli2` | 126 issues in 31 files — identical to baseline | Test count derivation: `main` is 1586, and this branch adds 11 — 10 in the new `bench_common_tests.rs` plus `hyperd_identity_guard_accepts_a_live_engine` — for 1597. The markdownlint findings for both changed Markdown files (`docs/BENCHMARK_GUIDE.md`, `hyperdb-api/CHANGELOG.md`) were diffed against `upstream/main`'s copies and are identical rule-for-rule, so this branch adds no new findings to the existing backlog.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the non-Node findings left open when
1.0.0-rc.1shipped, from the three adversarial reviews of #250 plus items discovered while cutting the release. Plan:docs/superpowers/plans/2026-09-05-post-rc-cleanup.md.Every item was re-verified against
mainbefore starting, rather than carried over from review text. Node findings are deliberately deferred so the N-API surface lands as one reviewable group.The two with real weight
Arrow decode errors were silently discarded (
authenticated_client.rs). Both label lookups iterated batches withif let Ok(batch) = batch_result, so a corrupt batch produced a map covering only what happened to decode — indistinguishable from "this table defines no labels", the worst failure for metadata used to render UI. The sameif letchain swallowed schema mismatches via itsdowncast_reftuple. The#[expect]waivers that documented this as deliberate were removed during the edition-2024 let-chain sweep, taking the recorded intent with them.Both now propagate. Safe: the only callers are in a salesforce example that already uses
unwrap_or_default(). Also fixes a latent panic —RecordBatch::column(1)panics out of bounds, so a one-column batch would have aborted. The two functions had byte-identical bodies, so the logic is extracted intoparse_label_pairs, fixing it once and dropping ~50 lines. Five new tests build real Arrow IPC streams and assert a truncated stream and a non-TEXT column are errors, which is precisely what the old code returnedOkfor.A user-facing command that didn't exist. The
HYPERD_PATH is not seterror told users to runcargo run -p hyperd-bootstrap -- download— a package gone since the rename, failing withpackage(s) not found in workspace. It's the first thing a new user sees whenHYPERD_PATHis unset. Verified both directions: the old form errors, the new one prints the CLI help.Coverage holes found while fixing
cfg-gated code is stripped before lints run and clippy only ran ubuntu, so "all 127collapsible_ifsites fixed" was Linux-scoped — one survivor sat inprocess.rs's#[cfg(windows)]Named Pipe branch. Addedwindows-latestto the clippy matrix and fixed it. macOS is deliberately omitted (sharescfg(unix)with the Linux leg).hyperdb-compile-check's tests. It declares its own[workspace], so--workspacecan't see it, and every CI reference is acargo checkor publish step. Its 13 tests — including registry logic changed in this branch — had never executed. Added to CI andmake test.make testcovered 3 of 8 crates while CI covers 7, making the widely-quoted "1519 passed" look like whole-workspace coverage. Now mirrors CI: 1586.hyperdb-compile-check— the one gate proving "builds on Red Hat's toolchain with no rustup" never covered a published crate.Hardening and accuracy
protocwas unzipped into/usr/localas root with no integrity check. Now sha256-verified against a pinned digest (upstream publishes no checksum file, so it's computed and recorded with its refresh command). Validated both directions locally.concurrencygroup, unlike every other workflow, so pushes stacked multi-minute container runs.hyperdb-compile-check's lint table had drifted from the root (warnvsdeny); it satisfiesdenywith no code changes.AGENTS.mdEditor Setup was wrong on both points (workspace-wide edition 2024, andrust-toolchain.tomlnow provisions rust-analyzer).hyperdb-compile-checkgot theCHANGELOG.mdit never had; AGENTS.md's list is now nine crates.One correction to a review finding
The review flagged the benchmark aggregation cell
~1 K/sas wrong, arguing the measured value is 199/s. Re-measuring showed the cell was right: 199/s is the 100M figure, while at 10M the workload is 10 rows in 0.007s ≈ 1.4 K/s. It was unverifiable, not incorrect. Left as-is.Verification
fmt,clippy -D warnings(workspace and the out-of-workspace crate),docover 8 crates,deny,audit, andcargo +1.88 check --lockedon both manifests all exit 0.make testis 1586 passed / 0 failed.