docs(board): post-merge governance for #459 (helix PR_ARC + LATEST_STATE table row)#460
Conversation
Board-hygiene follow-up for the merged helix PR (#459, merge ef35ff1): the PR-number-specific entries that can only be added once the PR has a number and a merge commit. - PR_ARC_INVENTORY: prepend the #459 arc entry (Added/Locked/Deferred/ Docs/Confidence) for crates/helix. - LATEST_STATE: add the #459 row to the "Recently Shipped PRs" table. The in-commit board updates (LATEST_STATE blockquote, STATUS_BOARD D-HELIX-1, EPIPHANIES E-HELIX-OVERLAP, TECH_DEBT TD-HELIX-OVERLAP-1, AGENT_LOG) already landed with #459. https://claude.ai/code/session_013rjF2Dvo1DnBACpbpYSffE
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 715dc0e999
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| **Added:** | ||
| - `crates/helix` (NEW standalone crate, zero-dep default, `edition 2021`, empty `[workspace]`, root `Cargo.toml` `exclude`) — the Place/Residue codec from `crates/helix/KNOWLEDGE.md`. Modules: `constants` (φ/γ/e + M=17/stride=4/ln 17), `placement` (`HemispherePoint::lift` — √u equal-area hemisphere), `curve_ruler` (`CurveRuler` — stride-4-over-17 arc from the HHTL place offset), `fisher_z` (`Similarity::{fisher_z,hyperbolic_depth}` — arctanh), `quantize` (`RollingFloor` — 256-palette, occupancy-drift + floor-version stamp), `distance` (`DistanceLut` — metric-safe 256×256 L1), `residue` (`ResidueEncoder::encode → ResidueEdge` 3-byte endpoint pair; `distance_adaptive` metric-safe / `distance_heuristic` non-metric), `prove` (2-D discrepancy companion to `jc::weyl`), `simd` (optional `ndarray-hpc` batch Fisher-Z via `ndarray::simd::simd_ln_f32`). | ||
| - `ndarray-hpc` feature (optional `dep:ndarray` → the AdaWorldAPI fork at `../../../ndarray`; the default build needs none of it). |
There was a problem hiding this comment.
Avoid claiming the default build needs no ndarray checkout
In a clean checkout where the sibling ../../../ndarray repo is absent, the documented default command still fails before feature selection because Cargo loads the optional path dependency manifest (failed to read /workspace/ndarray/Cargo.toml). This makes the new PR arc misleading for anyone trying to reproduce the stated zero-dep/default validation without the update-script-provided sibling checkout; either the doc should state that the checkout is still required for manifest resolution, or the dependency wiring needs to avoid a mandatory local path.
Useful? React with 👍 / 👎.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR updates two board-level documentation files to record the completion of PR ChangesPR
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…ray is mandatory") Two converging directives: 1. codex P2 (#460): an optional `path` dependency does not make the default build self-contained — Cargo reads every dependency manifest (incl. optional) during resolution, so `ndarray = { path = "../../../ndarray", optional = true }` makes a clean checkout without the sibling fail before feature selection. The "default build needs no ndarray checkout" claim was false. 2. Directive: "ndarray is mandatory for lance-graph" (it is The Foundation). Fix: ndarray is now a mandatory, non-optional GIT dependency: ndarray = { git = "https://github.com/AdaWorldAPI/ndarray.git", branch = "master", default-features = false, features = ["std"] } - git source resolves the manifest remotely (no sibling checkout needed); - non-optional, so the `ndarray-hpc` feature is removed; - src/simd.rs is now single-impl (always ndarray::simd, no scalar fallback; ndarray does its own AVX-512/AVX2/scalar dispatch internally). No import cycle: helix is standalone (own [workspace] + root exclude) and the fork is self-contained (only internal subcrate path deps, no lance-graph back-dependency). [patch] is the wrong tool here (the workspace's [patch.crates-io] ndarray is separately ineffective: 0.17.2 can't satisfy the lance-index crates.io 0.16.1). Verified: 63 unit + 6 doctests green, clippy -D warnings + fmt clean (git source patched to the local master checkout for the in-sandbox build; github fetch is validated by CI). Board: PR_ARC #459 Correction + E-HELIX-NDARRAY-MANDATORY + LATEST_STATE + STATUS_BOARD D-HELIX-1. https://claude.ai/code/session_013rjF2Dvo1DnBACpbpYSffE
Post-merge board-hygiene follow-up for the merged helix PR (#459, merge
ef35ff1) — the two PR-number-specific entries that can only be added once the PR has a number + merge commit:PR_ARC_INVENTORY.md— prepend the feat(helix): golden-spiral Place/Residue codec (zero-dep + optional ndarray-hpc) #459 arc entry (Added / Locked / Deferred / Docs / Confidence) forcrates/helix.LATEST_STATE.md— add the feat(helix): golden-spiral Place/Residue codec (zero-dep + optional ndarray-hpc) #459 row to the "Recently Shipped PRs" table.The in-commit board updates (LATEST_STATE blockquote, STATUS_BOARD
D-HELIX-1, EPIPHANIESE-HELIX-OVERLAP, TECH_DEBTTD-HELIX-OVERLAP-1, AGENT_LOG) already landed with #459. Docs-only — no code changes.https://claude.ai/code/session_013rjF2Dvo1DnBACpbpYSffE
Generated by Claude Code
Summary by CodeRabbit
Note: This release contains only internal documentation updates with no user-facing feature changes.