build: derive pure-simdjson version from go.mod and enforce NOTICE alignment - #51
build: derive pure-simdjson version from go.mod and enforce NOTICE alignment#51tazarov wants to merge 19 commits into
Conversation
- Resolve the selected pure-simdjson module version with go list - Keep the verified bootstrap fetch pinned to that derived version
- Attribute the 1,023/1,024 boundary to NewSIMDParser's default configuration - Preserve the stdlib comparison and explicit parser-parity warning
- remove stale adapter release attribution - document upstream and adapter depth configuration limits
- derive the expected pure-simdjson version from the module graph - run the NOTICE pin guard through lint
- validate all four required NOTICE line shapes - reject missing and suffixed dependency version tokens
- Cover full-file stale version detection and replacements\n- Assert invisible-character diagnostics and CI step ordering
- Resolve effective replacement versions from the module graph\n- Scan all NOTICE references and expose byte-escaped failure context
- Add a dedicated lint-job NOTICE alignment step\n- Preserve the existing validator and golangci-lint commands
- Assert unversioned local replacements fail closed\n- Require an actionable effective-version diagnostic
- Remove the `uses a bytewise locale` subtest, which asserted an exact Makefile source substring instead of guard behavior - Rewriting `export LC_ALL=C` as the equivalent `LC_ALL=C; export LC_ALL` still rejects the ZWSP mutation yet failed that assertion - Coverage is retained by `invisible version character is escaped`, which runs the guard under LC_ALL=en_US.UTF-8 and catches a missing LC_ALL=C
…E dump - Record that the version scan treats pure-simdjson as the only versioned attribution in NOTICE.md, so a second vendored dependency needs a wider scan - Filter the fail_notice dump to pure-simdjson and semver-token lines, reusing $version_pattern so the dump cannot drift from the scan - Retitle the diagnostic banner, which no longer shows the whole file
- Record mutation battery results and dump-narrowing measurements - Log pre-existing golangci-lint backlog and BSD/GNU sed locale nuance as deferred
Removes the staticcheck QF1001 finding introduced by this branch. The negated conjunction is rewritten as a disjunction of the inverse comparisons; the preceding guard already ensures all indices are non-negative, so semantics are unchanged.
The test declares the pure-simdjson module path as a string literal to build replace directives in scratch go.mod copies. simd-isolation-check grep-matches the quoted path and requires a simdjson build tag, so the literal is split to avoid the false positive without weakening the check or tagging a test that must run in default builds.
|
@claude PTAL |
|
Claude finished @tazarov's task in 3m 25s —— View job Review completeTodo list
Overall this is a well-scoped, carefully engineered guard — good use of One real finding:
|
There was a problem hiding this comment.
Pull request overview
This PR removes hardcoded pure-simdjson version references by deriving operational versions from the Go module graph, and adds CI + Makefile enforcement to keep NOTICE.md’s legally-required tag pins aligned with the effective module version.
Changes:
- Derive the SIMD bootstrap tool version at runtime from
go list -min CI, avoiding workflow pins that Dependabot can’t update. - Add a
make check-notice-versionguard (and CI step) that validatesNOTICE.md’s canonical pins and rejects any other semver tokens that drift. - Update docs/changelog/source comments to describe default parser behavior rather than pinning stale versions; add regression coverage for the NOTICE guard.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
Derives bootstrap version via go list -m; adds explicit NOTICE alignment step in the lint job. |
Makefile |
Adds check-notice-version guard; wires it into lint; documents the single-attribution scan constraint. |
notice_version_guard_test.go |
New regression suite that runs the real Make target in a temp copy and asserts CI step ordering. |
docs/simd-deployment.md |
Rewords nesting-limit section to describe adapter defaults and clarifies max-depth configurability limits. |
parser_simd.go |
Updates nesting-limit comment to avoid pinning a specific upstream tag. |
CHANGELOG.md |
Removes stale version pin from the Unreleased adapter entry. |
.planning/STATE.md |
Updates planning state timestamps and activity notes. |
.planning/quick/260730-pcc-polish-notice-version-guard-drop-brittle/deferred-items.md |
Records out-of-scope findings (pre-existing lint backlog, locale nuance). |
.planning/quick/260730-pcc-polish-notice-version-guard-drop-brittle/260730-pcc-SUMMARY.md |
Summarizes polish work on guard diagnostics/tests. |
.planning/quick/260730-pcc-polish-notice-version-guard-drop-brittle/260730-pcc-PLAN.md |
Plan for polish tasks on the NOTICE guard. |
.planning/quick/260730-kny-address-notice-version-guard-review-find/260730-kny-SUMMARY.md |
Summarizes CI wiring + guard hardening + regressions. |
.planning/quick/260730-kny-address-notice-version-guard-review-find/260730-kny-PLAN.md |
Plan for addressing NOTICE guard review findings. |
.planning/quick/260730-ije-address-stale-pure-simdjson-version-refe/260730-ije-VERIFICATION.md |
Verification report for stale-reference + NOTICE guard + doc updates. |
.planning/quick/260730-ije-address-stale-pure-simdjson-version-refe/260730-ije-SUMMARY.md |
Summary of NOTICE alignment and SIMD depth guidance task. |
.planning/quick/260730-ije-address-stale-pure-simdjson-version-refe/260730-ije-PLAN.md |
Plan for removing stale pins and adding NOTICE alignment guard. |
.planning/quick/260730-ftv-issue-49/260730-ftv-VERIFICATION.md |
Verification report for deriving CI version and scoping docs. |
.planning/quick/260730-ftv-issue-49/260730-ftv-SUMMARY.md |
Summary for Issue #49 resolution work. |
.planning/quick/260730-ftv-issue-49/260730-ftv-RESEARCH.md |
Research notes backing the CI + docs changes. |
.planning/quick/260730-ftv-issue-49/260730-ftv-PLAN.md |
Plan for the Issue #49 changes. |
.planning/quick/260730-ftv-issue-49/260730-ftv-CONTEXT.md |
Context and locked decisions for Issue #49 execution. |
Comments suppressed due to low confidence (3)
notice_version_guard_test.go:36
- This subtest bakes in both the current module version (v0.1.7) and an assumed replacement version (v0.1.8). Hard-coding the current version means a future pure-simdjson bump will force edits outside go.mod; derive the current effective version from the copied module instead and use it in the replace directive / NOTICE edits.
t.Run("replacement version is effective", func(t *testing.T) {
dir := copyNoticeGuardInputs(t)
appendFileText(t, filepath.Join(dir, "go.mod"), "\nreplace "+pureSIMDJSONModule+" v0.1.7 => "+pureSIMDJSONModule+" v0.1.8\n")
replaceFileText(t, filepath.Join(dir, "NOTICE.md"), "v0.1.7", "v0.1.8")
runNoticeVersionGuard(t, dir)
})
notice_version_guard_test.go:48
- This replace directive is pinned to "v0.1.7". If the required module version changes, the test will start failing even though the guard itself remains correct. Use the effective version from
go list -mon the copied module when constructing the replace directive.
t.Run("unversioned local replacement fails", func(t *testing.T) {
dir := copyNoticeGuardInputs(t)
localModule := filepath.Join(dir, "local-pure-simdjson")
if err := os.Mkdir(localModule, 0o700); err != nil {
t.Fatalf("Mkdir(%s) error = %v", localModule, err)
}
if err := os.WriteFile(filepath.Join(localModule, "go.mod"), []byte("module "+pureSIMDJSONModule+"\n"), 0o600); err != nil {
t.Fatalf("WriteFile(%s/go.mod) error = %v", localModule, err)
}
appendFileText(t, filepath.Join(dir, "go.mod"), "\nreplace "+pureSIMDJSONModule+" v0.1.7 => ./local-pure-simdjson\n")
notice_version_guard_test.go:59
- This test mutates a hard-coded heading string ("## pure-simdjson v0.1.7"), so it will break on any future dependency bump even if the guard behavior is unchanged. Build the heading using the effective module version from the copied module instead.
t.Run("invisible version character is escaped", func(t *testing.T) {
dir := copyNoticeGuardInputs(t)
replaceFileText(t, filepath.Join(dir, "NOTICE.md"), "## pure-simdjson v0.1.7", "## pure-simdjson v\u200b0.1.7")
output := runNoticeVersionGuardFailure(t, dir)
requireOutputContains(t, output, "NOTICE.md", "\\342\\200\\213")
})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| t.Run("stale version anywhere in NOTICE fails", func(t *testing.T) { | ||
| dir := copyNoticeGuardInputs(t) | ||
| appendNotice(t, dir, "\nHistorical note: previously pinned v0.1.2 before the current pin.\n") | ||
|
|
||
| output := runNoticeVersionGuardFailure(t, dir) | ||
| requireOutputContains(t, output, "NOTICE.md", "v0.1.7") | ||
| }) |
| run: | | ||
| pure_simdjson_version="$(go list -m -f '{{.Version}}' github.com/amikos-tech/pure-simdjson)" | ||
| go run "github.com/amikos-tech/pure-simdjson/cmd/pure-simdjson-bootstrap@${pure_simdjson_version}" fetch |
Summary
Closes #49.
pure-simdjsonwas pinned in several places Dependabot cannot reach, and the docs had drifted tov0.1.4whilego.modwas atv0.1.7. This removes the hardcoded pins where a derived value works, and adds a CI-enforced guard for the one place a literal version is genuinely required (NOTICE.md, where license attribution must name a specific tag).The net effect: a
go get -uofpure-simdjsoncan no longer leave a stale, externally-visible version claim behind without failing CI.Changes
CI derives the bootstrap version from
go.mod—.github/workflows/ci.ymlReplaces
pure-simdjson-bootstrap@v0.1.7with a version resolved at runtime viago list -m. This uses the MVS-selected version rather than ago.modgrep, so it stays correct if a transitive requirement ever selects higher.Stale
v0.1.4references removed —parser_simd.go,CHANGELOG.mdThe
simdNestingDepthLimitcomment and the Unreleased changelog entry both named a version two bumps behind. Both now describe the parser's default configuration rather than pinning a tag, so they can't go stale on a patch bump.Docs scoped to the parser default —
docs/simd-deployment.mdThe nesting-limit claim now describes
pure-simdjson's default maximum depth instead of a specific version. Also documents that upstream exposesWithMaxDepthbutNewSIMDParser()takes no options, so callers cannot configure depth through this adapter — previously the doc invited readers to hunt for a knob that doesn't exist.New
make check-notice-versionguard —Makefile, wired into.github/workflows/ci.ymlNOTICE.mdmust pin an exact tag for license attribution, so it can't be de-pinned. Instead it's now verified against the effective module version:replacedirectives, and rejects unversioned local replacementsLC_ALL=Cgolangci-lint, mirroring the existingcheck-validator-markerspatternRegression suite —
notice_version_guard_test.goCopies the guard's inputs into a temp dir and runs the real Make recipe. Covers stale versions anywhere in the file, effective replacements, unversioned local replacements, invisible characters, and CI step ordering.
Verification
LC_ALL=en_US.UTF-8 go test ./...— all packages passmake check-notice-version— passes on a clean treegolangci-lint— 50 findings, identical to theorigin/mainbaseline; zero new findings introducedNOTICE.mdbyte-identical to its committed state after all mutation testingHistorical note: previously pinned v0.1.2 before the current pin.v0.0.9lineBundled some-other-lib v2.3.4 under Apache-2.0.Key Decisions
NOTICE.mdkeeps its literal version pins. License attribution must name a specific tag, so de-pinning was not an option. Enforcing alignment in CI was the alternative.The guard allows exactly one versioned attribution. Any semantic-version token in
NOTICE.mdthat isn't the expectedpure-simdjsonversion fails the check. This is deliberate — it's what catches stale historical references — but it means adding a second vendored dependency with its own version requires widening the scan. Recorded in a comment above the target.LC_ALL=Cis load-bearing, not cosmetic. In a UTF-8 locale, BSDawkcompares by collation weight, so a zero-width space is invisible to==. For a guard whose entire job is byte-exact matching, the locale is part of the contract.Notes for reviewers
goconstfindings are pre-existing onorigin/mainand unrelated to this branch. They appear locally under golangci-lint 2.12.2; CI pins v2.11.4, which is whymainis green..planning/are included, matching this repo's existing convention (e.g. c316c70).