B1.scaffold: Go module + cobra + version + snapshot infrastructure - #116
Conversation
|
Deployment failed with the following error: Learn More: https://vercel.com/thrillmot-s-projects?upgradeToPro=build-rate-limit |
|
Claude finished @thrillmot's task in 5m 48s —— View job 🐛 Clud Bug reviewThis round: 1 critical · 1 minor · 0 resolved from prior · 0 still open Found: 1 🔴 / 1 🟡 / 0 🟣 Per-skill scan
🔴 [critical-issues-only]: Double error printing in Reasoning
🟡 [critical-issues-only]: ReasoningOnly Skills referenced: [critical-issues-only, evidence-based-review, respect-existing-conventions] |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
🐛 Clud Bug reviewThis round: 1 critical · 1 minor · 0 resolved from prior · 0 still open Found: 1 🔴 / 1 🟡 / 0 🟣 Per-skill scan
Critical findings🔴 [critical-issues-only]: Cobra double-prints errors before returning; main.go then prints again, producing duplicate stderr on every error path (cmd/logmind/main.go:20). Reasoningroot.go sets SilenceUsage: true but NOT SilenceErrors, so cobra's default ExecuteC() calls c.PrintErrln("Error:", err.Error()) to stderr before returning. main.go then calls fmt.Fprintln(os.Stderr, err) — the comment acknowledges cobra prints its own errors yet still re-prints. Fix: add SilenceErrors: true to the root command in root.go to suppress cobra's built-in printing and let main.go be the single authoritative printer. Minor findings🟡 [critical-issues-only]: make snapshot passes -update to all test binaries; future packages without flag.Bool("update",...) registration will fail with flag provided but not defined (Makefile:52). ReasoningPKG covers ./cmd/... ./internal/... — any new test package that doesn't declare var update = flag.Bool("update", ...) will cause make snapshot to exit 2 with 'flag provided but not defined: -update'. Currently harmless (only internal/cli has tests and registers the flag), but the PR positions this scaffold as the pattern for future waves. A comment in the target or a naming convention (e.g. -run TestSnapshot) would prevent confusing failures. Skills referenced: [critical-issues-only, evidence-based-review, respect-existing-conventions] |
… output, fragile snapshot flag)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude finished @thrillmot's task in 2m 5s —— View job
|
🐛 Clud Bug reviewThis round: 0 critical · 0 minor · 0 resolved from prior · 0 still open Found: 0 🔴 / 0 🟡 / 0 🟣 Per-skill scan
Skills referenced: [critical-issues-only, evidence-based-review, respect-existing-conventions] |
…119) ## Summary Wave B3 of the logmind Go rewrite — ports the derived-doc generators and the rebase wrapper. - 4 new subcommands: `timeline`, `file-structure`, `tree`, `rebase` - 4 new internal packages: `internal/config`, `internal/decisions`, `internal/timeline`, `internal/tree` - 2 new helpers on `internal/gitcli`: `DefaultBranch` (5-step search mirror), `RunCaptured` (stderr-exposing variant for rebase) - `Makefile` `SNAPSHOT_PKGS` extended with `internal/timeline/...` + `internal/tree/...` Built on top of B1 + B2 (PRs #116 + #117). Targets `v1-go-rewrite`. ## Byte-identical output vs Python v0.6.14 Verified against the actual logmind repo (112 decisions across 6 months, ~120KB unbounded tree): | Command | Result | |---|---| | `logmind timeline` (brief, default) | BYTE-IDENTICAL (1647 bytes) | | `logmind timeline --full` | BYTE-IDENTICAL (27407 bytes) | | `logmind file-structure` (depth 2) | BYTE-IDENTICAL (4180 bytes) | | `logmind file-structure --max-depth 0` (unbounded) | BYTE-IDENTICAL (121729 bytes) | | `logmind file-structure --max-depth 1` | BYTE-IDENTICAL (975 bytes) | | `logmind file-structure --max-depth 3` | BYTE-IDENTICAL | | `logmind timeline --check` (clean) | BYTE-IDENTICAL | | `logmind file-structure --check` (clean) | BYTE-IDENTICAL | | `logmind tree` | BYTE-IDENTICAL | | `logmind timeline --check` (stale) | BYTE-IDENTICAL (exit 1) | | `logmind rebase` (failure path on dirty WT) | BYTE-IDENTICAL | Confirmation method: `diff <(venv/bin/logmind <cmd>) <(bin/logmind <cmd>)` against `/Users/ludlow/logmind` itself — the largest real fixture available. ## Snapshot tests (49 total + 7 golden fixtures) - **`internal/timeline/`** — 4 goldens covering brief/full × elision-vs-not, plus the empty/single-newline property tests - `brief-mixed.golden` — 3 months (4-entry elision, 2-entry verbatim, 1-entry singleton) - `brief-singular-elision.golden` — exactly 3 entries → `... 1 more decision ...` singular noun - `brief-two-elided-months.golden` — two elided months back-to-back for inter-month spacing - `full-mixed.golden` — full mode pins lack of count suffix on month headers - **`internal/tree/`** — `generate-file-structure.golden` pins the template head + tail bytes - **`internal/cli/`** — `timeline_stdout_brief.golden`, `timeline_stdout_full.golden` for end-to-end stdout shape - 23 cli tests + 26 package tests, all green via `make test` ## Brief-mode byte-identical proof Brief-mode month grouping is the highest-risk algorithm — Python uses `lines = [HEADER, ""]` then `"\\n".join(lines)`, which produces three newlines between the `---` footer of the header and the first `## YYYY-MM`. Go's `strings.Builder` byte-level approach would emit only two unless you mirror the join semantics precisely. Resolution: assemble a `[]string` mirror of Python's `lines` and use `strings.Join(lines, "\\n")` rather than emitting bytes directly. The trailing `lines.append("")` gives the final `\\n` after the last entry. See `internal/timeline/timeline.go` comments for the trace. Verified against the actual `docs/timeline.md` which carries the brief-mode elision lines for 6 months of real history. ## Tree-walk byte-identical proof Tree walk uses pure Go (`filepath.WalkDir` not invoked — explicit `os.ReadDir` per frame for sort control). Sort key matches Python: `(not is_dir, name.lower())` → directories first then case-insensitive alphabetical. Pattern matching is path-aware (matches full relative path, components, and basename). One known semantic divergence: `filepath.Match`'s `*` does NOT cross `/`, but Python's `fnmatch.fnmatchcase`'s `*` DOES. For the patterns logmind actually ships (`DEFAULT_IGNORES` + typical `.gitignore`), this is invisible — patterns are either segment-level (`*.pyc`, `__pycache__`) or fully literal (`site/.next`). Documented in `patternSetMatches`. Verified at depth 0/1/2/3 against the logmind repo itself which is the most realistic fixture I have (it has `site/.next/`, `venv/`, `__pycache__/`, `.git/`, mixed casing, et al). ## Known divergence vs Python v0.6.14 `--check` without `--write`: - Python: prints error + exits **2** - Go: prints byte-identical error + exits **1** The stdout message is unchanged so consumers diffing output see no difference. The exit-code divergence is a documented known issue. Closing it would require an `ErrSilentExit2` sentinel in `cmd/logmind/main.go` (deferred to a coordinated cross-wave change — B4 lands first and is a candidate). ## Open questions for human reviewer 1. **Exit-code divergence**: keep as documented divergence, or land the `ErrSilentExit2` sentinel as a coordinated patch alongside B4? 2. **Glob semantics divergence**: the `*-vs-/` divergence is theoretical but real. Want a custom fnmatch port that's `*=anything-including-/` strict, or accept the practical equivalence? 3. **Rebase `--no-fetch`**: Python exposes `--no-fetch` so tests can rebase against a fixture without an `origin/`. I ported it for parity; any objection? 4. **B4 coexistence**: this PR is independent of B4 work happening in parallel on `feat/go-b4-agents`. The B4 branch will need to add `root.AddCommand(newAgentsCmd())` next to the B3 wires. ## Design decisions - **Brief-mode algorithm**: assemble `[]string` mirror + `strings.Join` rather than byte-level emission. See comment in `internal/timeline/timeline.go:106-119`. - **Tree walker is pure Go**: never shell out to system `tree(1)` even when available. Consuming repos get identical output across OSes. See `internal/tree/tree.go:1-22`. - **Config loader strategy**: typed `Config` struct with `yaml:` tags; user keys overlay defaults leaf-by-leaf via `yaml.Unmarshal` into a pre-populated default struct. Mirrors Python's `_deep_update` shape without recursive Map traversal in Go. See `internal/config/config.go:118-140`. - **`-1 sentinel` for unbounded depth**: CLI's `--max-depth 0` translates to internal `-1` so `Render` can use a simple `maxDepth >= 0 && depth >= maxDepth` guard. Documented at the function boundary. - **DefaultBranch 5-step search**: extends `internal/gitcli` (not duplicated in `internal/rebase`) because B5 (init) and B4 (agents in some paths) will also want it. ## Test plan - [x] `make build && make test` — all packages green - [x] `make snapshot` regenerates 7 golden files without drift - [x] Byte-identical diff vs Python v0.6.14 across 8+ invocation shapes on the logmind repo itself - [x] `--check` stale + clean paths verified - [x] `rebase` failure paths verified (not-a-repo, detached HEAD, refusing-self, fetch fail, rebase fail) - [ ] CI matrix (Go 1.22+) — will run on push - [ ] Manual sanity check by reviewer on a consuming repo (e.g. tokenomics) to confirm the merge driver still converges 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: thrillmot <thrillmot@users.noreply.github.com>
1 participant
Summary
Wave B1.scaffold of the Python → Go rewrite. Lands the bare-minimum
foundation that every subsequent wave builds on:
go.mod(modulegithub.com/thrillmade/logmind, Go 1.22+) +go.sum.cmd/logmind/main.go(thin entry shim) →internal/cli/(cobra root + subcommand registration) +internal/version/(version constants).logmind --versionANDlogmind versionboth emit theprotocol-contract line:
internal/cli/testdata/with ashared
-updateflag (driven bymake snapshot). Five tests coverthe format from three angles: in-process function call, in-process
cobra tree exec, and built-binary subprocess.
Makefile:make build,make test,make snapshot,make verify-parity(placeholder),make tidy,make clean..github/workflows/go-test.yml: matrix CI (ubuntu-latest/macos-latest,Go 1.22 + 1.24) gating only the
v1-go-rewritebranch + its featurebranches. Existing
test.yml(pytest matrix) continues to gatemainuntouched.
on
v1-go-rewrite; PyPI keeps shipping frommain.Out of scope (intentionally)
--version. Notinit, notlog, notshow, notsearch. Each gets its own wave PR.src/logmind/(Python) is untouched. Both implementations coexistthrough the rewrite.
mainbranch is untouched. Wave PRs targetv1-go-rewrite; thefinal cutover PR
v1-go-rewrite → mainbecomesv1.0.0.pkg/directory. Everything isinternal/until SPEC.md locksa public Go API.
make verify-parityis a stub. The first wave that ports areal subcommand fills it in by diffing Go stdout vs Python v0.6.14
stdout for the same args.
Snapshot test pattern (load-bearing for future waves)
Every future wave that ports a subcommand SHOULD follow this pattern:
internal/cli/testdata/<command>.golden— rawstdout bytes the command should produce.
TestXxx_InProcesstest that drivesNewRootCmd()withSetArgs([...]), captures the buffer, and diffs against the golden.TestXxx_Subprocesstest that builds the real binary (viathe
repoRootFromCallerhelper) and execs it for an extra byte-levelguarantee.
-updateflag somake snapshotregeneratesevery golden in one pass.
The
--versiontest ininternal/cli/version_test.gois thecanonical example.
Decisions baked in (recorded via
logmind log)flag— ecosystem ubiquity(kubectl, gh, hugo, helm) + group/subcommand shape mirrors Python
click closely.
internal/rather thanpkg/— v1.0's public surface IS thebinary; no other repo should import logmind's guts as a Go library
yet, and
internal/enforces that at compile time. Promotepackages later if SPEC.md locks a public API.
parity gate against Python v0.6.14 a one-line diff (Python stdout
is the future golden for the matching Go command). No JSON
wrapping, no
cmpopts, just bytes.--versionformat, NOT byte-identical to Python's clickdefault (
logmind, version 0.6.14). v1.0 publishes a stablemachine-parseable line including the spec version so downstream
tooling (clud-bug, tokenomics) can detect protocol skew.
Open questions for review
has no config to read. The first wave that needs config can decide.
Flagging here so a reviewer can pre-redirect if there's already a
preferred library on the roadmap.
time of writing. Happy to bump to 1.23 if there's a stdlib feature
the rewrite already wants.
site/node_modules/flatted/golang/ships a third-party Gopackage inside the docs site's npm tree.
make testscopes to./cmd/... ./internal/...to skip it cleanly; flagging in case thesite/node_modules tree should be moved out of the working copy or
gitignored entirely.
Test plan
make buildproducesbin/logmind../bin/logmind --versionand./bin/logmind versionboth printlogmind 1.0.0-dev (spec 0.1.0-draft).make testgreen (5 tests pass ininternal/cli).make snapshotis a no-op when goldens already match.go-testjob runs on this PR (Ubuntu × Go 1.22, Ubuntu × Go 1.24,macOS × Go 1.22) and stays green.
test.ymlpytest matrix continues to pass (Go changesdon't touch Python paths).