Refine research doc and pin golangci-lint to restore CI - #91
Merged
Conversation
Revise docs/toward-a-universal-grammar.md to capture design decisions from exploration of the engine formalism and tokenisation story. §7.1 (Parsing Algorithm): rewritten to commit to a GLL-family top-down CFG parser with a layered DFA fast path, CFG-unordered alternation, and a declarative disambiguation vocabulary (.prefer, .avoid, .assoc, .priority, .longest). Grammars must be provably deterministic or explicitly disambiguated; silent PEG-order disambiguation is gone. GLL is chosen over ALL(*) for its native scannerless composition, native left recursion, top-down mental model continuity, and clean path to data-dependent extensions (§7.2). §3.2 (Integrated Tokenization): rewritten to use existing named-capture syntax (name=expr) as labels, so the tok::label mechanism needs no new labeling construct. Added subsection 3.2.5 on automatic regularity detection. The .lex directive becomes optional strictness rather than a required mode marker — "lexer" disappears as a surface-language concept. The :: operator is shown to be safe under regex unification because : is already grammar-level syntax (delimited repetition). §5.1 (Formal Expressiveness): updated to reference the GLL-family engine choice and §7.1. §6.1 (Related Work / Scannerless Parsing): added Meerkat/Iguana (Afroozeh & Izmaylova) as the closest engine-level neighbour — GLL plus data-dependent grammar support — distinguished from SDF/Rascal as the closest philosophical neighbour. §7.3 (Implementation of Proposed Extensions): added item 5 for the engine migration, noted as precondition for positional constraints and grammar composition in full generality. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Commit 3a4dd96 (update workflows, May 2023) removed the explicit version: v1.48 pin from the golangci-lint-action invocation, letting it float to "latest". Because the Go workflow only triggers on pull_request — not on push — no CI run has exercised this workflow since the pin was removed. This PR is the first to do so, and two-plus years of golangci-lint drift have surfaced at once: gosec G115 integer-overflow checks, depguard rules-format evaluation, and config deprecations (exportloopref, govet.check-shadowing, github-actions output format). None of these are this PR's content (it is docs-only). Restoring the explicit v1.48 pin returns CI to the state the .golangci.yml config was written against. Addressing the underlying code-smell issues surfaced by newer golangci-lint versions is worth doing, but as a separate deliberate piece of work rather than as a side-effect of a docs change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
marcelocantos
added a commit
that referenced
this pull request
Apr 11, 2026
Three targets identified during PR #91's push cycle, tracked durably rather than left as transcript notes: - 🎯T1: CI lints cleanly under current golangci-lint without a version pin. PR #91 restored a v1.48 pin as a stopgap after two-plus years of golangci-lint drift surfaced at once. Removing the pin cleanly requires addressing the underlying findings (gosec G115 integer overflows in wbnf/compile.go, an errcheck on state.Write, an unparam on testWbnfFile, and a .golangci.yml depguard config in an obsolete format) plus removing deprecated config keys. - 🎯T2: No open high-severity Dependabot alerts on master. GitHub flagged 1 high-severity alert during the PR #91 push, unrelated to the PR's content. Likely resolvable by a targeted `go get -u` plus `go mod tidy`. - 🎯T3: CI workflow actions run on Node 24-compatible versions. CI annotations flagged Node.js 20 deprecations on actions/checkout@v3, actions/setup-go@v3, and golangci/golangci-lint-action@v3. GitHub forces Node 24 on 2026-06-02 and removes Node 20 on 2026-09-16, so there's time but the bump is inevitable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
marcelocantos
added a commit
that referenced
this pull request
Apr 11, 2026
Three targets identified during PR #91's push cycle, tracked durably rather than left as transcript notes: - 🎯T1: CI lints cleanly under current golangci-lint without a version pin. PR #91 restored a v1.48 pin as a stopgap after two-plus years of golangci-lint drift surfaced at once. Removing the pin cleanly requires addressing the underlying findings (gosec G115 integer overflows in wbnf/compile.go, an errcheck on state.Write, an unparam on testWbnfFile, and a .golangci.yml depguard config in an obsolete format) plus removing deprecated config keys. - 🎯T2: No open high-severity Dependabot alerts on master. GitHub flagged 1 high-severity alert during the PR #91 push, unrelated to the PR's content. Likely resolvable by a targeted `go get -u` plus `go mod tidy`. - 🎯T3: CI workflow actions run on Node 24-compatible versions. CI annotations flagged Node.js 20 deprecations on actions/checkout@v3, actions/setup-go@v3, and golangci/golangci-lint-action@v3. GitHub forces Node 24 on 2026-06-02 and removes Node 20 on 2026-09-16, so there's time but the bump is inevitable. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
marcelocantos
added a commit
that referenced
this pull request
Apr 11, 2026
The Generate-Tag workflow had been silently cutting minor version tags on every push to master (v0.38.0 from PR #91, v0.39.0 from PR #92) because its config never matched the commented intent. The workflow's comment documented "Generate tag if commit message contains #major or #patch or #patch" but the underlying anothrNick/github-tag-action defaults to DEFAULT_BUMP=minor, so every push produced an auto-bump regardless of commit content. The issue had been latent for years because no-one had merged anything in a long while; it surfaced this week when two unrelated PRs cut v0.38.0 and v0.39.0 on merges that weren't releases in any meaningful sense. Pare back to rely on the /release skill for deliberate releases — version bumping, release notes, CI, Homebrew tap, tag creation, and GitHub release publishing as explicit user actions. This matches the "only the user can initiate releases" principle from global CLAUDE.md. The go.yml workflow is kept unchanged — it remains the CI gate that /push watches before merging. Knock-on: 🎯T3 (CI workflow actions run on Node 24-compatible versions) shrinks from two workflow files to one, since the deleted workflow's actions/checkout@master and actions/checkout@v3 references are gone with it. Target context updated accordingly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
marcelocantos
added a commit
that referenced
this pull request
Apr 11, 2026
The Generate-Tag workflow had been silently cutting minor version tags on every push to master (v0.38.0 from PR #91, v0.39.0 from PR #92) because its config never matched the commented intent. The workflow's comment documented "Generate tag if commit message contains #major or #patch or #patch" but the underlying anothrNick/github-tag-action defaults to DEFAULT_BUMP=minor, so every push produced an auto-bump regardless of commit content. The issue had been latent for years because no-one had merged anything in a long while; it surfaced this week when two unrelated PRs cut v0.38.0 and v0.39.0 on merges that weren't releases in any meaningful sense. Pare back to rely on the /release skill for deliberate releases — version bumping, release notes, CI, Homebrew tap, tag creation, and GitHub release publishing as explicit user actions. This matches the "only the user can initiate releases" principle from global CLAUDE.md. The go.yml workflow is kept unchanged — it remains the CI gate that /push watches before merging. Knock-on: 🎯T3 (CI workflow actions run on Node 24-compatible versions) shrinks from two workflow files to one, since the deleted workflow's actions/checkout@master and actions/checkout@v3 references are gone with it. Target context updated accordingly. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
marcelocantos
added a commit
that referenced
this pull request
Apr 11, 2026
Addresses 🎯T3. Updates the three Node 20-deprecated pins in go.yml to their current latest major versions: - actions/checkout: v3 → v6 - actions/setup-go: v3 → v6 - golangci/golangci-lint-action: v3 → v9 Pre-existing Node 20 deprecation annotations (flagged on every CI run since PR #91) should disappear. GitHub Actions is forcing Node 24 by default on 2026-06-02 and removing Node 20 from runners on 2026-09-16, so these bumps land well inside the grace window. Known risk: golangci-lint-action@v9 may not support downloading the v1.48 golangci-lint binary we pinned in PR #91. If CI fails on that, the fix is part of 🎯T1 (remove the v1.48 pin entirely after migrating the .golangci.yml config and fixing the code-level findings that newer golangci-lint versions flag). That work is coming next. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
marcelocantos
added a commit
that referenced
this pull request
Apr 11, 2026
Removes the v1.48 pin from PR #91 and addresses all findings that newer golangci-lint versions surface against this codebase. Config migration (.golangci.yml): - Migrated from v1 to v2 format via `golangci-lint migrate`. - Removed depguard (never configured; was triggering blanket import- denial errors under depguard's rules-based format). - Removed errcheck.check-blank=true, which was flagging the idiomatic `_, _ = state.Write(...)` in parser/scanner.go's Format method — standard Go style for fmt.Formatter implementations. - Removed dead gocritic settings (linter isn't enabled). - Deprecated exportloopref, govet.check-shadowing, and the github-actions output format are dropped by the migration. Code fixes: - cmd/test.go: testWbnfFile no longer returns an always-nil error (unparam). The single caller is updated. - wbnf/compile.go: four //nolint:gosec comments silence G115 integer overflow warnings on the byte-escape parsing in parseString. The octal and \x cases are genuinely safe (ParseInt bitSize=8 constrains the value); the \u and \U cases have a latent truncation bug that nobody has ever hit because no grammar in the repo uses these escape forms. The latent bug is tracked separately — the nolint comments point at 🎯T4 for follow-up. - parser/parser.go, parser/terms.go, wbnf/compile.go, wbnf/cutpoints.go: four prealloc fixes converting `[]T{}` / `var x []T` patterns to `make([]T, 0, len(...))` where the target length is statically known from the loop bound. Workflow (.github/workflows/go.yml): - Removed the `version: v1.48` pin on golangci-lint-action. The action's default now installs the current golangci-lint, which runs cleanly against the migrated config and the code fixes. Combined with the action version bumps in a97465c (🎯T3), the Go workflow now runs on Node 24-compatible actions with no version pins and no outstanding lint findings. Local verification: `go build ./...`, `go test ./...`, and `golangci-lint run` all clean. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
marcelocantos
added a commit
that referenced
this pull request
Apr 11, 2026
* Bump CI actions to Node 24-compatible major versions Addresses 🎯T3. Updates the three Node 20-deprecated pins in go.yml to their current latest major versions: - actions/checkout: v3 → v6 - actions/setup-go: v3 → v6 - golangci/golangci-lint-action: v3 → v9 Pre-existing Node 20 deprecation annotations (flagged on every CI run since PR #91) should disappear. GitHub Actions is forcing Node 24 by default on 2026-06-02 and removing Node 20 from runners on 2026-09-16, so these bumps land well inside the grace window. Known risk: golangci-lint-action@v9 may not support downloading the v1.48 golangci-lint binary we pinned in PR #91. If CI fails on that, the fix is part of 🎯T1 (remove the v1.48 pin entirely after migrating the .golangci.yml config and fixing the code-level findings that newer golangci-lint versions flag). That work is coming next. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Resolve 🎯T1: lint cleanly under current golangci-lint Removes the v1.48 pin from PR #91 and addresses all findings that newer golangci-lint versions surface against this codebase. Config migration (.golangci.yml): - Migrated from v1 to v2 format via `golangci-lint migrate`. - Removed depguard (never configured; was triggering blanket import- denial errors under depguard's rules-based format). - Removed errcheck.check-blank=true, which was flagging the idiomatic `_, _ = state.Write(...)` in parser/scanner.go's Format method — standard Go style for fmt.Formatter implementations. - Removed dead gocritic settings (linter isn't enabled). - Deprecated exportloopref, govet.check-shadowing, and the github-actions output format are dropped by the migration. Code fixes: - cmd/test.go: testWbnfFile no longer returns an always-nil error (unparam). The single caller is updated. - wbnf/compile.go: four //nolint:gosec comments silence G115 integer overflow warnings on the byte-escape parsing in parseString. The octal and \x cases are genuinely safe (ParseInt bitSize=8 constrains the value); the \u and \U cases have a latent truncation bug that nobody has ever hit because no grammar in the repo uses these escape forms. The latent bug is tracked separately — the nolint comments point at 🎯T4 for follow-up. - parser/parser.go, parser/terms.go, wbnf/compile.go, wbnf/cutpoints.go: four prealloc fixes converting `[]T{}` / `var x []T` patterns to `make([]T, 0, len(...))` where the target length is statically known from the loop bound. Workflow (.github/workflows/go.yml): - Removed the `version: v1.48` pin on golangci-lint-action. The action's default now installs the current golangci-lint, which runs cleanly against the migrated config and the code fixes. Combined with the action version bumps in a97465c (🎯T3), the Go workflow now runs on Node 24-compatible actions with no version pins and no outstanding lint findings. Local verification: `go build ./...`, `go test ./...`, and `golangci-lint run` all clean. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update targets: retire 🎯T1 and 🎯T3, introduce 🎯T4 🎯T1 (CI lints cleanly under current golangci-lint without a version pin): achieved by commits a97465c and a2cd03e. Config migrated from v1 to v2, underlying lint findings fixed, v1.48 pin removed. Local build, tests, and golangci-lint run are all clean. 🎯T3 (CI workflow actions run on Node 24-compatible versions): achieved by commit a97465c. actions/checkout@v3 → v6, actions/setup-go@v3 → v6, golangci/golangci-lint-action@v3 → v9. No more Node 20 deprecation annotations expected. 🎯T4 (parseString correctly handles \x, \u, \U, and octal escapes): new target capturing a latent bug in wbnf/compile.go's string-literal escape parsing, discovered while adding //nolint:gosec comments as part of 🎯T1. The \x/\u/\U cases have off-by-one slice offsets that would panic on any real use, and \u/\U additionally truncate Unicode code points to a single byte via WriteByte. Untested code paths — no grammar in the repo uses these escapes — so the bugs have never been observed in practice. Fix is small but out of scope for a lint cleanup pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
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.
Revise docs/toward-a-universal-grammar.md to capture design decisions
from exploration of the engine formalism and tokenisation story.
§7.1 (Parsing Algorithm): rewritten to commit to a GLL-family top-down
CFG parser with a layered DFA fast path, CFG-unordered alternation, and
a declarative disambiguation vocabulary (.prefer, .avoid, .assoc,
.priority, .longest). Grammars must be provably deterministic or
explicitly disambiguated; silent PEG-order disambiguation is gone. GLL
is chosen over ALL(*) for its native scannerless composition, native
left recursion, top-down mental model continuity, and clean path to
data-dependent extensions (§7.2).
§3.2 (Integrated Tokenization): rewritten to use existing named-capture
syntax (name=expr) as labels, so the tok::label mechanism needs no new
labeling construct. Added subsection 3.2.5 on automatic regularity
detection. The .lex directive becomes optional strictness rather than a
required mode marker — "lexer" disappears as a surface-language concept.
The :: operator is shown to be safe under regex unification because :
is already grammar-level syntax (delimited repetition).
§5.1 (Formal Expressiveness): updated to reference the GLL-family engine
choice and §7.1.
§6.1 (Related Work / Scannerless Parsing): added Meerkat/Iguana
(Afroozeh & Izmaylova) as the closest engine-level neighbour — GLL plus
data-dependent grammar support — distinguished from SDF/Rascal as the
closest philosophical neighbour.
§7.3 (Implementation of Proposed Extensions): added item 5 for the
engine migration, noted as precondition for positional constraints and
grammar composition in full generality.
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com