Refactor: v5 API Overhaul and Architecture Rewrite#38
Merged
Conversation
- Implement a new flat operation model for patches and Hybrid Logical Clocks (HLC) for causality. - Add code generation via cmd/deep-gen for optimized, reflection-free operations. - Move the legacy reflection-based diff/patch engine to internal/engine as a fallback. - Update all subpackages (cond, crdt, resolvers) to the v5 module path and integrate the new API. - Re-implement core synchronization helpers and examples using type-safe field selectors. - Significant performance improvements verified with benchmarks (up to 14x for apply).
- Unexport internal reflection and selector helpers in the root package. - Rename internal symbols to remove redundant "V5" suffix for idiomaticity. - Enhance 'deep-gen' to produce self-contained, warning-free generated code. - Remove dead code in 'internal/engine' (old merge logic and unused unmarshalers). - Regenerate all examples using the improved generator. - Move internal test types (User, Detail) to dedicated test files. - Add robust settability checks to reflection-based fallback logic.
- Renamed TestV5_* to Test* in engine_test.go, diff_test.go, and patch_test.go. - Verified all tests pass. - v5_test.go was previously removed in favor of functional test files.
- Renamed user_test.go to test_user_test.go. - Renamed user_deep_test.go to test_user_deep_test.go. - Regenerated test_user_deep_test.go to ensure consistency.
…e tests - Renamed root package from v5 to deep for idiomaticity. - Moved Text CRDT implementation entirely to the crdt package and added full convergence logic (normalize, MergeTextRuns). - Reorganized test models into internal/testmodels and refactored root tests to package deep_test to resolve import cycles. - Redistributed tests from coverage_test.go into functional test files and removed it. - Cleaned up root directory by removing NEXT_STEPS.md and redundant test model files. - Updated all examples to work with the new package structure and aliased imports. - Enhanced generator to use deep as the default package name.
- Path[T,V].String() value receiver was attempting to memoize into a copy; remove the futile local assignment since resolvePathInternal already caches globally via pathCache - Fix TOCTOU race in resolvePathInternal: re-check the cache under the write lock before scanning so concurrent callers don't double-scan - scanStructInternal now skips fields tagged json:"-", which previously produced spurious paths like "/-" - Collapse ParsePath's two identical branches into a single ParseJSONPointer call (dead-branch removal)
- Bump go directive to 1.24 - Add log.go: exported Logger (*slog.Logger) and SetLogger helper - Replace fmt.Printf in cond/condition_impl.go with slog.Default().Info
- Fix nil check order in Apply - Stamp op.Strict from Patch.Strict at apply time; Operation.Strict is json:"-" - Fix Merge: later HLC wins; tie-break other-wins; sort output for determinism - Dispatch global condition to generated EvaluateCondition when available - Replace fmt.Printf with Logger.Info / slog in engine.go and patch_ops.go - Fix nil-source map diff emits OpAdd not OpReplace - Add Patch.IsEmpty() - Add Ge/Le condition constructors - Add Builder.Move and Builder.Copy - Add FromJSONPatch[T] full round-trip - Add LWW[T].Set(v, ts) bool - Add Op* constants for all condition operators
- Replace WriteString(Sprintf(...)) with text/template for structural skeleton - Move type-specific code fragments into FuncMap helper functions - Add pkgPrefix field to Generator struct for precise import analysis - needsStrings only true for struct fields and map[string]* collections - needsRegexp only true when there are scalar (non-struct, non-collection) fields - Rename generated evaluateCondition to exported EvaluateCondition - Emit Logger.Info instead of fmt.Printf in generated code
crdt.Text.Diff takes a value receiver (Text), not a pointer (*Text). The dispatcher only checked Diff(*T), so Text fell through to reflection and Apply never populated the target. Add a second check for Diff(T).
audit_logging: use Diff() instead of Builder so op.Old captures actual previous field values rather than nil. config_manager: deep-copy Features map before mutating to prevent v1 aliasing; use patch.Reverse() for rollback instead of re-diffing against a shared-reference v1.
API changes (breaking from any pre-release usage):
- Diff[T] now returns (Patch[T], error) instead of Patch[T]
- Patch.Condition field renamed to Patch.Guard
- WithCondition method renamed to WithGuard
- Logger changed from package-level var to Logger() *slog.Logger
(backed by atomic.Pointer; concurrent-safe with SetLogger)
Bug fixes:
- ApplyError implements Unwrap() []error for errors.Is/As support
- Builder.Where merges with And() instead of silently overwriting
- Strict mode in reflection fallback now also covers OpRemove
- Generator strict check handles float64 Old values after JSON roundtrip
- fromPredicateInternal uses comma-ok assertions (no panic on bad input)
- Diff no longer silently swallows reflection engine errors
Generator:
- Default output is now {type}_deep.go instead of stdout
- All _deep.go files regenerated with updated generator
Package structure:
- cond/ moved to internal/cond/ (was an unexposed dead package)
Documentation:
- README code block fixed; go:generate usage added; JSON float64 note added
- CHANGELOG.md created with full v4→v5 breaking change list
- Path.Index/Key docs note the any value-type loss
- Register[T] docs explain why []T and map[string]T are registered
Tests / benchmarks:
- BenchmarkApplyReflection added alongside BenchmarkApplyGenerated
- All callers updated for new Diff signature with proper error handling
…silent errors with slog
…o.mod minimum version
…ency - Remove business_rules (no library features; redundant with policy_engine) - Rename custom_types → lww_fields; rewrite to demonstrate LWW[T] per-field registers with v5.Merge (genuine library feature, not misleading dead code) - Rename key_normalization → struct_map_keys (accurate name for the feature) - Rewrite crdt_sync to use crdt.CRDT[T] Edit/ApplyDelta instead of raw patches - Fix audit_logging: Tags map[string]bool triggers Add/Remove/Replace ops - Fix config_manager: use v5.Copy instead of manual map copying - Fix websocket_sync: value-type Player for addressable Apply; regenerate - Fix state_management: patch-based undo with Reverse() instead of snapshots - Fix json_interop: add RFC 6902 output via ToJSONPatch for comparison - Fix http_patch_api: fix import order, simplify output - Standardize all output to use --- SECTION --- headers consistently
Remove API that added surface area without value:
- Patch.Metadata map[string]any field
- NewPatch[T]() constructor (zero-value Patch[T]{} is idiomatic)
- CondLog = "log" constant and evaluateCondition "log" case
- Log[T,V] condition constructor (wrong semantics — side-effectful predicate)
- OpTest opkind and testPatch with all its methods (RFC 6902 "test" op was
never generated and explicitly skipped in the v5 diff path)
Fix all examples:
- Replace import alias `v5 "github.com/brunoga/deep/v5"` with natural `deep`
package name throughout; update all `v5.` call sites to `deep.`
- Replace all `deep.NewPatch[T]()` calls with `deep.Patch[T]{}`
CRDT v5 port (crdt/crdt.go):
- Replace all internal/engine, internal/cond, internal/resolvers/crdt
dependencies with the public deep v5 API (deep.Diff, deep.Apply, deep.Copy)
- Delta.patch is now deep.Patch[T] (flat ops) instead of engine.Patch[T]
- ApplyDelta implements LWW inline by iterating delta.patch.Operations;
effective local time = max(write clock, tombstone)
- Merge implements state-based LWW inline using other.clocks/tombstones
- Text type gets a convergent bypass in ApplyDelta — always merges via
MergeTextRuns, never filtered by LWW clocks (concurrent inserts/deletes
must all be applied regardless of timestamp ordering)
- Remove textPatch type and init() registration; no engine dependency remains
Delete internal/resolvers/crdt (LWWResolver, StateResolver now dead).
Text JSON roundtrip (crdt/text.go):
- ApplyOperation handles []interface{} (JSON-decoded arrays) by re-marshaling
through encoding/json before calling MergeTextRuns
Keyed-slice-aware path navigation (internal/core/path.go):
- Navigate, Set, and Delete now detect slices whose element type carries a
deep:"key" tagged field and use key-value lookup instead of treating
numeric path segments as positional array indices
- This closes the semantic gap between deep.Diff (which generates paths like
/Friends/101 using the element key) and deep.Apply (which previously
treated 101 as an array index, causing "index out of bounds" errors)
- Non-keyed slices retain existing positional behaviour
text_sync example rewritten to use Insert/Delete/MergeTextRuns API.
fieldApplyCase (ApplyOperation): for crdt.Text fields, emit a delegation to t.Field.ApplyOperation(op) with op.Path = "/" instead of a direct type assertion. This invokes MergeTextRuns (convergent merge) rather than overwriting the field, which is semantically correct for CRDT text. diffFieldCode (Diff): remove the bogus nil guard for IsText fields. Taking the address of a struct field (&t.Bio) is never nil; crdt.Text is a slice with a valid zero value, and Text.Diff already handles empty input. Only pointer fields need a nil guard. Regenerate internal/testmodels/user_deep.go to pick up both fixes.
engine.go: Resolve returns a reference directly into the target struct, so calling Delete on the source field zeroed val in place before it could be written to the destination — producing an empty string in both fields. Fix by copying the resolved value into a fresh reflect.Value before deleting the source. examples/move_detection: switch from %+v to %q per field so empty strings are visible and the before/after state is unambiguous.
All deletions are code that became unreachable during the v5 overhaul: internal/cond/ — deleted Only used by internal/engine's basePatch.cond and patch serialization, both of which are removed in this commit. internal/engine/patch_serialization.go — deleted PatchToSerializable/PatchFromSerializable/RegisterCustomPatch were internal plumbing never reachable through the public deep.Patch[T] API. internal/engine/patch.go Remove WithCondition (depended on internal/cond), MarshalSerializable/ MarshalJSON/UnmarshalJSON (depended on serialization), and the cond field on typedPatch. Remove internal NewPatch[T]() (redundant with zero value). internal/engine/patch_ops.go Remove basePatch struct and the conditions()/setCondition() interface methods that existed solely to support per-op condition serialization. internal/core/path.go Remove PathPart.Equals() (no external callers) and ToReflectValue() (only called from deleted patch_serialization.go). internal/core/util.go Remove InterfaceToValue() (only called from deleted patch_serialization.go). cmd/deep-gen/main.go Remove deref() helper (no callers remain after generator refactor). internal/testmodels/user.go Remove NewUser() constructor (no callers in test suite). internal/engine/patch_test.go Remove two commented-out test functions for deleted functionality.
Introduce Op as a small value type returned by typed operation constructors (Set, Add, Remove, Move, Copy). Builder.With(ops ...Op) is the single chain-preserving entry point for all typed ops, replacing the old pattern of package-level functions that took *Builder as their first argument. Per-op conditions (If/Unless) are now attached to the Op value before passing to With, eliminating the stateful "modifies last op" Builder methods If and Unless. Rename deep.Copy[T] to deep.Clone[T] (consistent with slices.Clone, maps.Clone) to free the Copy name for the patch-op constructor Copy[T,V](from, to Path[T,V]) Op. Remove from Builder: Set, Add, Remove, Move, Copy, If, Unless.
Remove deep.SetLogger()/deep.Logger() global mutable state that caused race conditions when multiple libraries configure logging independently. Replace with Apply(..., WithLogger(l)) functional option; slog.Default() is used when no logger is provided. The logger is threaded through the generated ApplyOperation(op Operation, logger *slog.Logger) interface so per-field OpLog operations receive the caller-supplied logger.
Demonstrate OpLog + WithLogger alongside the existing diff-based audit trail pattern. Add an Observability section to the README showing how to embed log operations in a patch and route them to a slog.Logger.
…on.Timestamp Remove Operation.Timestamp *hlc.HLC and the engine's reflection-based LWW skip logic. The deep core package no longer imports crdt/hlc. Move LWW[T] and its Set method to the crdt package where it belongs alongside CRDT[T]. Callers who want LWW semantics over plain struct fields use crdt.LWW[T] directly; full causal synchronization uses crdt.CRDT[T]. Merge conflict resolution falls back to "other wins" when no ConflictResolver is provided — timestamp-based resolution is no longer part of the core engine.
Run go fmt ./... across the codebase. Remove two genuinely unreachable functions from internal/engine: - Differ.Diff(a, b any): superseded by the generic DiffUsing[T] path - MustDiffUsing[T]: no callers anywhere in the codebase Public API flagged by deadcode (Ne, In, Matches, Type, WithLogger) and encoding/json interface implementations (Delta.MarshalJSON/UnmarshalJSON) are intentionally kept.
Five targeted API improvements, all breaking: - Unexport Selector type: users pass func(*T)*V closures to Field() and never need to name the selector type. Removes it from the public surface. - Replace Path.Index/Path.Key with At[]/MapKey[] free functions: the old methods returned Path[T,any], silently losing the element type. The new generic free functions constrain the receiver to ~[]E and ~map[K]V respectively, so the full type is preserved at the call site. - Rename Builder.Where to Builder.Guard: consistent with Patch.Guard field and Patch.WithGuard method. No reason for the naming inconsistency. - Rename FromJSONPatch to ParseJSONPatch: idiomatic Go uses Parse for decoding from bytes (time.Parse, url.Parse, etc.). From is not standard. - Replace Patch.WithStrict(bool) with Patch.AsStrict(): the bool parameter was meaningless — WithStrict(false) was always a no-op. AsStrict() makes the intent explicit and removes the dead parameter.
- Replace ApplyOperation/EvaluateCondition on generated types with a single exported Patch(p Patch[T], logger *slog.Logger) error method; applyOperation and evaluateCondition are now unexported - Add ApplyOpReflection[T] as the cross-package reflection fallback called by generated Patch for unhandled operations (e.g. slice index) - Apply same Patch / applyOperation pattern to crdt.Text - Remove type alias 'type Condition = core.Condition' from deep package; generated code now imports core directly - Rename generated Copy() → Clone() to match deep.Clone() public API - Remove from public API: Register[T], gob init, Cond* constants, ApplyConfig/NewApplyConfig (logger passed directly through Patch) - Unexport core.ParseApply → parseApply (only used internally) - Move core files from internal/core to public core/ package - Fix OpLog handling in generated applyOperation (was hitting "unsupported root operation" error at path "/") - Rename misnamed generated files to match first type in each example - go fmt ./...
- Slim core/ to condition-only: move cache, copy, equal, path, tags, util back to internal/core/ — only Condition, EvaluateCondition, CheckType, ToPredicateInternal, and FromPredicateInternal remain public - Move Operation struct to internal/engine/operation.go; re-export as type alias in deep package (type Operation = engine.Operation) - Move ApplyOpReflection[T] to internal/engine/apply_reflection.go; generated code now calls _deepengine.ApplyOpReflection — no longer part of the public API - Add cmd/deep-gen golden-file test to catch template regressions - Add float64 coercion warning to Apply doc comment - Add /deep-gen to .gitignore
- selector.go: add nil check for selector return value — panics with a
descriptive message ("use &u.Field, not u.Field") instead of silently
producing a garbage offset that resolves to an empty path
- CHANGELOG.md: remove references to deleted API (Register[T], Logger,
SetLogger, Timestamp field, ApplyOperation/EvaluateCondition/Copy
methods); update Condition.Apply → Condition.Sub
- README.md: fix stale "Generated ApplyOperation methods" → "Generated
Patch methods"
- Initialize nil pointer fields before running the selector so that paths through pointer-typed struct fields (e.g. &n.Inner.Value where Inner is *InnerType) resolve correctly instead of panicking on nil dereference. - Add cycle detection in initializePointers via an in-progress type set so self-referential types (e.g. linked lists) do not cause infinite recursion / stack overflow. - Restore per-selector caching via sync.Map keyed by the selector's function pointer, recovering the O(1) repeated-call performance of the original offset-based cache. - Add cycle guard in findPathByAddr (visited address set) to prevent infinite walks if a pointer cycle survives initialization. - Add TestSelectorNestedPointer and TestSelectorCircularType.
- Remove all references to Gob serialization and the non-existent Register function; Patch is JSON-only. - Fix Merge doc comment: it deduplicates ops by path, with other winning on conflict — there is no HLC timestamp comparison. - README: remove JSON/Gob claim; show At() in Quick Start builder example; add Patch Utilities section (Reverse, AsStrict); add ParseJSONPatch to Standard Interop section. - CHANGELOG: fix Merge row description; add missing public API entries (At, MapKey, ParseJSONPatch, and all Patch[T] methods).
- Fix headline perf claim from 26x to 15x (matches benchmark table max) - Replace At+Add slice example with MapKey+Add map example — OpAdd on slices sets by index, not inserts; the map variant is unambiguous - Fix Apply signature in CHANGELOG to include opts ...ApplyOption
…ELOG Rename: - Condition.ToPredicateInternal() → Condition.ToPredicate() - core.FromPredicateInternal() → core.FromPredicate() Both were public functions in a public package with "Internal" in their names — a contradiction. The new names are symmetric, accurate, and don't signal false danger to callers. Callers in patch.go updated. CHANGELOG additions: - ConflictResolver interface (required to customise Merge) - CRDT package: CRDT[T], NewCRDT, Delta[T], LWW, Text, MergeTextRuns - crdt/hlc package: Clock, NewClock, HLC - core package: full listing (Condition, EvaluateCondition, CheckType, ToPredicate/FromPredicate, operator constants)
Promotes the public condition API to a properly-named package: github.com/brunoga/deep/v5/condition instead of the misleadingly generic core/ path. All callers updated to use condition.Condition directly — no type alias. Generated *_deep.go files regenerated.
… names condition.CondEq/Ne/Gt/... → condition.Eq/Ne/Gt/... condition.EvaluateCondition → condition.Evaluate Package name already carries the context; the prefixes were noise.
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
1 participant
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.
Refactor: v5 API Overhaul and Architecture Rewrite
Description
This PR introduces the highly anticipated v5.0.0 architecture overhaul. It contains significant structural and API-level changes, transitioning from a recursive tree model to a flat operation model. It also introduces code generation for dramatic performance improvements, revamps the CRDT and Condition systems, and modernizes logging.
Note: This release contains breaking changes from v4.
Key Changes
Architecture & Core
Packages & Modules
Cleanups & Quality of Life