Refactor: break up the cli god-package + code-smell cleanups#6
Merged
Conversation
Every command except init and clean opened with the same ~10-line
block: EnsureGitRoot, print + wrap on failure, EnsureInitDone, print +
wrap on failure, plus SilenceUsage. It was copy-pasted across seven
RunE functions, and docs/spec/preconditions.md explicitly calls for
"one central way to check init is done; same message everywhere."
Extract RequireInitializedRepo(cmd) — it runs both checks, prints the
user-facing message, and returns a SilentError so callers collapse to
`gitRoot, err := RequireInitializedRepo(cmd); if err != nil { return err }`.
Net -24 lines and the shared behavior now lives in one place.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TBMA7dNfAQn86m59P64dsS
Two small duplications had crept in: - recall.go built the "$1,$2,...,$N" placeholder list and args slice inline in three batch loaders. Extract sqlInClause(sessionIDs). - db.nullIfEmpty/nullIfZero and sync_remote's sqlNullIfEmpty/ sqlNullIfZero were identical NULL-coalescing helpers in two packages. Export them from db (NullIfEmpty/NullIfZero) — the storage layer is where they belong — and have the raw INSERT in sync_remote reuse them, dropping the copies. Behavior-preserving cleanup only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TBMA7dNfAQn86m59P64dsS
The Packages list omitted scrub/ (secret redaction + path anonymization that runs on every checkpoint before any DB write — security-relevant and worth surfacing), and the Core CLI list omitted lsa_cache.go (the LSA query-projection cache). Add both, and note the new RequireInitializedRepo precondition helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TBMA7dNfAQn86m59P64dsS
cmd/rekal/cli was a 25-file god-package; recall.go alone was ~1,050 lines mixing the cobra command with the whole hybrid-search engine. Move the engine into a new cli/search package — hybrid BM25+LSA+Nomic ranking, signal weighting, conversation grouping, snippet extraction, the facet batch loaders, and the LSA query-projection cache (lsa_cache.go -> search/projection.go). recall.go shrinks to ~60 lines of command orchestration: open, migrate, and auto-rebuild the index DB, call search.Run, marshal JSON. The package exposes a small surface — Run, Filters, Result, SessionDetail, Output, StoreLSAProjection — with everything else unexported. The three affected callers (root, index, sync) switch to the search.* names, and the recall/batch/projection tests move into the package. Output shape and JSON tags are unchanged, so recall output is byte-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TBMA7dNfAQn86m59P64dsS
…ckage Completes the cli package breakup. The command files kept doing three unrelated jobs: cobra command wiring, the wire-format export/import pipeline, and low-level git plumbing. Pull the latter two out: - cli/transport: encode checkpoints to the orphan-branch wire format and decode them back (export.go + import.go + sync_remote.go, plus ensureOrphanBranch/commitWireFormat). Sits above codec, db, and gitx. - cli/gitx: the thin git-plumbing helpers (rev-parse, show, hash-object, config, rekal/<email> branch name) that both the command layer and transport need. Putting them in a shared lower package is what lets transport exist without a commands<->transport import cycle. cli now holds only command definitions and their orchestration. No behavior change — pure code movement; the exported transport/gitx surface is called from push/sync/init and the export regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TBMA7dNfAQn86m59P64dsS
Two small smells surfaced while reviewing the split packages: - The ULID generator closure (a shared math/rand entropy source plus a ulid.MustNew wrapper) was copy-pasted three times — once in checkpoint and twice in transport. Extract cli/ids.NewULIDFunc(), which returns the same reuse-one-generator closure the batch loops expect, and drop the now-unused math/rand and oklog/ulid imports from both callers. - The wire filenames "rekal.body" and "dict.bin" were spelled out in seven places across init and transport. Give them names where the format is defined: codec.BodyFilename / codec.DictFilename. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TBMA7dNfAQn86m59P64dsS
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
2 participants
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.
Structural refactors and code-smell cleanups on top of v0.3.0. Behavior-preserving throughout — no logic changes, recall output byte-identical. CI + Lint green on the tip (
c9288ec).Package structure
cmd/rekal/cliwas a 25-file god-package. Broke it into focused packages with a clean dependency DAG:cli/search— the recall ranking engine (hybrid BM25+LSA+Nomic, signal weighting, conversation grouping, snippet extraction, LSA projection cache).recall.goshrank from ~1,050 lines to ~60 lines of command orchestration.cli/transport— wire export/import + orphan-branch commit (wasexport.go/import.go/sync_remote.go).cli/gitx— thin git plumbing (rev-parse, show, hash-object, config,rekal/<email>branch name). The shared lower layer that letstransportexist without acommands ↔ transportimport cycle.cli/ids— ULID generation.clinow holds only command definitions + orchestration.Code-smell cleanups
RunEs intoRequireInitializedRepo(spec already called for "one central way").db.NullIfEmpty/NullIfZero) and IN-clause placeholder building (sqlInClause).ids.NewULIDFunc()and the"rekal.body"/"dict.bin"literals (×7) intocodec.BodyFilename/codec.DictFilename.Docs
CLAUDE.mdto match the new package layout and fill prior gaps (scrub/, LSA cache).🤖 Generated with Claude Code
https://claude.ai/code/session_01TBMA7dNfAQn86m59P64dsS
Generated by Claude Code