feat(epaxos): conflict engine core with model-equivalence tests#2
Merged
Conversation
Drop govet enable-all; keep revive/gosec/exhaustive enabled for R9.
Make epaxos package lint-clean under the U1 strict config: errcheck on Tick paths, revive renames, intentional test nolints for G115/G304/exhaustive subsets, and leave production wire G115 checks intact.
Root golangci-lint does not load the nested module; add a second CI step and document the local dual-module lint commands in AGENTS.md.
Mirror parser.uvarint8 for bootstrap envelopes so oversized wire types cannot truncate into the valid BootstrapMessageType band.
Re-check depsCount against maxWireDeps before int conversion and arena slicing so a hostile evidence frame cannot panic the node.
MkdirAll/WriteFile already use restrictive 0o750/0o600; the G115 suppressions were false and would hide future mode regressions.
Standalone per-lane sparse radix max-seq index and per-key postings with fold summaries; property tests prove oracle equivalence and fold domination.
Reviewer's GuideIntroduce a standalone per-lane conflict engine with radix/posting data structures and property-style tests, while tightening protocol/message/bootstrap validation, improving error handling and security hardening across tests, examples, and tools, and wiring in stricter linting via golangci-lint and CI updates. Sequence diagram for conflict engine apply and foldRecord/advanceFoldsequenceDiagram
participant RawNode
participant conflictEngine
participant laneIndex
participant laneTree
participant keyLane
participant postingSet
RawNode->>conflictEngine: apply(prev *InstanceRecord, rec InstanceRecord)
conflictEngine->>conflictEngine: laneFor(rec.Ref)
conflictEngine->>conflictEngine: slotForRecord(rec)
conflictEngine->>conflictEngine: recordConflictEligible(rec)
conflictEngine->>conflictEngine: commandHasGlobalConflictScope(rec.Command.Kind)
conflictEngine->>conflictEngine: ensureLane(lane)
conflictEngine->>laneTree: set(rec.Ref.Instance, slot)
alt recordConflictEligible(rec) && !commandHasGlobalConflictScope(rec.Command.Kind)
loop for each key in rec.Command.ConflictKeys
conflictEngine->>conflictEngine: ensureKeyLane(rec.Ref.Conf, key, lane)
conflictEngine->>keyLane: postings.insert(rec.Ref.Instance)
end
end
RawNode->>conflictEngine: foldRecord(rec InstanceRecord)
conflictEngine->>conflictEngine: laneFor(rec.Ref)
conflictEngine->>conflictEngine: ensureLane(lane)
conflictEngine->>conflictEngine: remove(rec.Ref, rec)
conflictEngine->>postingSet: insert(rec.Ref.Instance)
conflictEngine->>conflictEngine: recordConflictEligible(rec)
alt recordConflictEligible(rec)
conflictEngine->>conflictEngine: addRetiredSeq(rec.Ref.Instance, rec.Seq)
alt commandHasGlobalConflictScope(rec.Command.Kind)
conflictEngine->>conflictEngine: update retiredGlobal
else non-global keys
loop for each key in rec.Command.ConflictKeys
conflictEngine->>conflictEngine: ensureKeyLane(rec.Ref.Conf, key, lane)
conflictEngine->>keyLane: update retiredFloor
end
end
end
RawNode->>conflictEngine: advanceFold(lane, through InstanceNum)
conflictEngine->>conflictEngine: ensureLane(lane)
conflictEngine->>postingSet: contains(instance)
conflictEngine->>postingSet: remove(instance)
conflictEngine->>conflictEngine: update folded
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Author
|
Closes #9 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="examples/kv/kv.go" line_range="122-128" />
<code_context>
func (b TimestampBounds) classify(ts uint64) timestampMatch {
switch b.mode {
+ case timestampLatest:
</code_context>
<issue_to_address>
**issue (bug_risk):** TimestampBounds.classify lacks a return for timestampLatest, which will not compile.
The new `case timestampLatest` branch only has a comment and does not return a `timestampMatch`, unlike the other cases and with no default/trailing return. With `mode == timestampLatest`, this path has no return and will not compile in Go. Add an explicit return (e.g., `timestampVisible` or the intended value) in this case.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
122
to
128
| func (b TimestampBounds) classify(ts uint64) timestampMatch { | ||
| switch b.mode { | ||
| case timestampLatest: | ||
| // All timestamps are visible under latest mode. | ||
| case timestampAtOrBefore: | ||
| if ts > b.max { | ||
| return timestampTooNew |
There was a problem hiding this comment.
issue (bug_risk): TimestampBounds.classify lacks a return for timestampLatest, which will not compile.
The new case timestampLatest branch only has a comment and does not return a timestampMatch, unlike the other cases and with no default/trailing return. With mode == timestampLatest, this path has no return and will not compile in Go. Add an explicit return (e.g., timestampVisible or the intended value) in this case.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a per-lane conflict engine using radix trees and posting sets to optimize conflict and attribute computation, replacing the previous O(all instances) scan machinery. It also adds extensive linting annotations, refactors error handling, and ensures exhaustive enum dispatching across the codebase. A review comment suggests a performance improvement in keyLaneSet by hoisting a map lookup out of nested loops to avoid redundant operations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+644
to
+664
| func (e *conflictEngine) keyLaneSet(conf ConfID, keys [][]byte, yield func(instanceLane) bool) { | ||
| for lane := range e.laneIndex { | ||
| if lane.conf != conf { | ||
| continue | ||
| } | ||
| for _, key := range keys { | ||
| byConf := e.byKey[conf] | ||
| if byConf == nil { | ||
| break | ||
| } | ||
| byLane := byConf[string(key)] | ||
| if byLane == nil || (*byLane)[lane] == nil { | ||
| continue | ||
| } | ||
| if !yield(lane) { | ||
| return | ||
| } | ||
| break | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The map lookup e.byKey[conf] only depends on conf, which is constant throughout the execution of keyLaneSet. Hoisting this lookup outside of the loops avoids repeating the lookup for every single lane and key, improving the query's efficiency.
func (e *conflictEngine) keyLaneSet(conf ConfID, keys [][]byte, yield func(instanceLane) bool) {
byConf := e.byKey[conf]
if byConf == nil {
return
}
for lane := range e.laneIndex {
if lane.conf != conf {
continue
}
for _, key := range keys {
byLane := byConf[string(key)]
if byLane == nil || (*byLane)[lane] == nil {
continue
}
if !yield(lane) {
return
}
break
}
}
}![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4295ec2bf8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+491
to
+495
| type conflictEngine struct { | ||
| laneIndex map[instanceLane]*laneIndex | ||
| byKey map[ConfID]map[string]*keyLanes | ||
| resident int | ||
| } |
There was a problem hiding this comment.
When this commit is used in production, this new engine is unreachable: RawNode still stores and queries the old conflicts/allConflicts/globalConflicts maps, and computeAttrsAt still falls back to scanning n.instances; there is also no VisitConflicts API exposed. This means the added O(log n) conflict structure only runs in conflict_engine_test.go, so workloads with many resident instances keep the old O(all instances) behavior and embedders cannot use the promised conflict query surface.
Useful? React with 👍 / 👎.
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.
Solution / proposal
Standalone sparse 64-ary radix per-lane conflict engine with per-key O(log) postings, foldRecord/advanceFold, retired floors, and property tests (oracle equivalence, fold domination).
Diagnosis addressed
D1 seq scan, D5 string keys, foundation for D2/D6 elimination.
Closes #
Summary by Sourcery
Introduce a per-lane conflict engine and related safety hardening while tightening validation, error handling, and linting across EPaxos, examples, and test harnesses.
Enhancements:
CI:
Documentation:
Tests:
Closes #9