chore!: move sequencers to pkg #2931

tac0turtle · 2025-12-15T14:39:08Z

Overview

This pr moves sequencers to pkg, its a nit, we should probably hold off on merging to avoid lots merge conflicts with da removal

github-actions · 2025-12-15T14:39:21Z

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
`✅ passed`	`⏩ skipped`	`✅ passed`	`✅ passed`	Jan 3, 2026, 3:44 PM

claude · 2025-12-15T14:39:25Z

Claude finished @tac0turtle's task in 3m 0s —— View job

Code Review Complete

I've performed a comprehensive code review of this PR. The refactoring successfully moves the sequencer implementations from the root sequencers/ directory to pkg/sequencers/, improving the project structure. The changes are well-executed overall with consistent updates across the codebase.

Tasks

Review project guidelines (CLAUDE.md)
Analyze PR diff and file changes
Verify import path consistency
Check documentation updates
Review code quality and architecture
Assess testing coverage
Provide final review feedback

Summary

This PR performs a structural refactoring with minimal functional changes:

18 files changed (mostly renames and import updates)
Core change: sequencers/* → pkg/sequencers/*
Impact: Import paths updated across all application entry points
Risk level: Low (mechanical changes, no logic modifications)

Detailed Findings

✅ What's Good

Consistent Import Path Updates
- All Go files correctly updated: apps/testapp/cmd/run.go:24-26, apps/grpc/cmd/run.go:26-28, apps/evm/cmd/run.go:25-27
- Internal package references updated: pkg/sequencers/based/sequencer.go:17, pkg/sequencers/single/sequencer.go:21
- No old import paths remain in the codebase
Documentation Alignment
- apps/grpc/README.md:154 correctly updated the sequencer documentation link
- pkg/sequencers/based/README.md and pkg/sequencers/single/README.md remain accurate
- Markdown linting fixes applied (commit d70f76e)
Proper .gitignore Addition
- Added /.cache to .gitignore:33 - good housekeeping
File Structure
- All files properly moved with git history preserved
- Directory structure now aligns with Go package conventions (implementation code in pkg/)
- Zero-dependency core/ package pattern maintained

⚠️ Issues Found

1. CRITICAL: Incorrect Documentation in CLAUDE.md

The CLAUDE.md file contains outdated and incorrect interface documentation that pre-dates this PR but should be fixed:

Lines 53-55 (Current - INCORRECT):

- **Executor** (`core/executor.go`) - Handles state transitions
- **Sequencer** (`core/sequencer.go`) - Orders transactions
- **DA** (`pkg/da/types`) - Data availability layer abstraction

Should be:

- **Executor** (`core/execution/execution.go`) - Handles state transitions
- **Sequencer** (`core/sequencer/sequencing.go`) - Orders transactions
- **DA** (`pkg/da/types`) - Data availability layer abstraction

Verification:

core/executor.go does NOT exist ❌
core/sequencer.go does NOT exist ❌
Actual files:
- core/execution/execution.go defines the Executor interface ✅
- core/sequencer/sequencing.go defines the Sequencer interface ✅

Impact: This will confuse developers trying to understand the codebase architecture.

2. CLAUDE.md Update in This PR

The PR did update CLAUDE.md (commit 69e52f6), but the update introduced the above error. The change from:

- **Executor** (core/executor.go)

to:

- **Execution** (`core/execution/execution.go`)

Was close, but has issues:

Changed "Executor" to "Execution" (inconsistent - the interface is named Executor)
The format is inconsistent with other entries

Gemini's review comment was correct - this needs fixing.

Code Quality Assessment

Architecture & Design

✅ Modular structure: Sequencer implementations properly isolated in pkg/
✅ Zero-dependency core: No changes to core interface contracts
✅ Clear boundaries: Implementation code separated from interface definitions
✅ Consistent patterns: Both sequencer types (based, single) follow same package structure

Error Handling & Resilience

✅ No changes to error handling logic
✅ Checkpoint and recovery mechanisms unchanged
✅ Context handling remains consistent

Security

✅ No security implications from this refactoring
✅ No exposed credentials or secrets
✅ No changes to authentication/authorization

Performance

✅ No performance impact (pure refactoring)
✅ No changes to concurrency patterns
✅ Import paths don't affect runtime performance

Testing

✅ Test files moved alongside implementation: pkg/sequencers/based/sequencer_test.go, pkg/sequencers/single/sequencer_test.go, etc.
✅ No test logic modified
⚠️ Cannot verify if tests pass without running them (no approval for make test)
ℹ️ CI shows: codecov reports no coverage change (60.14% before and after)

Documentation

✅ README files moved with code
✅ Internal documentation (READMEs) remains accurate
⚠️ CLAUDE.md has errors (as noted above)

Recommendations

Must Fix Before Merge

Fix CLAUDE.md interface documentation (lines 53-55)
- Correct file paths to match actual structure
- Use consistent naming (interface names, not package names)

Good to Have

Consider adding DA types path update
- Line 123 references core/da/types.go which should be pkg/da/types for consistency
- Verify actual location and update if needed
Verify all references
- Do a final grep for any missed references to old paths in comments or strings
- Check if any generated code might reference old paths

Notes

Merge timing: Author correctly notes this should wait to avoid conflicts with DA removal work
Semantic versioning: This is marked as chore!: (breaking change) - appropriate since import paths change
CI status: All checks passing ✅

Conclusion

This is a well-executed structural refactoring with one documentation issue that should be fixed. The code changes are mechanical, consistent, and low-risk. The import path updates are complete and correct.

Recommendation: Fix the CLAUDE.md documentation errors, then this PR is ready to merge (when DA removal conflicts are resolved).

Overall Quality: ⭐⭐⭐⭐ (4/5) - Would be 5/5 with documentation fix.

gemini-code-assist · 2025-12-15T14:39:39Z

Summary of Changes

Hello @tac0turtle, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request undertakes a structural refactoring by relocating the sequencer-related modules into a dedicated pkg directory. This change aims to enhance the overall organization of the codebase, making it more consistent with common Go project layouts. While a relatively minor change in functionality, it involves widespread updates to import paths and documentation to ensure all references align with the new module structure. The author notes that merging this PR might be best deferred to avoid conflicts with ongoing Data Availability (DA) removal efforts.

Highlights

Refactored Sequencer Module Location: The sequencers directory and its contents have been moved into a new pkg/sequencers path to improve project organization and adhere to Go's conventional package structure.
Updated Import Paths Across Codebase: All Go files referencing the sequencer modules have been updated to reflect the new pkg/sequencers import paths, ensuring consistency throughout the project.
Documentation and Configuration Alignment: Project documentation (CLAUDE.md, apps/grpc/README.md) and Dockerfile configurations have been adjusted to match the new module structure and updated interface definitions.
Gitignore Enhancement: The .gitignore file now includes /.cache to prevent caching artifacts from being committed to the repository.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request refactors the project structure by moving the sequencer implementations into the pkg directory, which is a good change for improving code organization. The changes are mostly mechanical, involving file moves and updates to import paths and documentation. These have been done correctly and consistently across the codebase. I've found one minor issue in the documentation that could be addressed to improve clarity for developers.

gemini-code-assist · 2025-12-15T14:41:30Z

CLAUDE.md

- **Executor** (core/executor.go) - Handles state transitions
- **Sequencer** (core/sequencer.go) - Orders transactions
- **DA** (core/da.go) - Data availability layer abstraction
+- **Execution** (`core/execution/execution.go`) - State transition interface and types


There seems to be a typo in the documentation for the Execution component. The interface defined in core/execution/execution.go is named Executor, but it's documented here as Execution. To avoid confusion for developers, this should be corrected to Executor.

Suggested change

- **Execution** (`core/execution/execution.go`) - State transition interface and types

- **Executor** (`core/execution/execution.go`) - State transition interface and types

codecov · 2025-12-15T14:45:05Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.73%. Comparing base (af5d6b1) to head (d70f76e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2931      +/-   ##
==========================================
- Coverage   58.86%   58.73%   -0.13%     
==========================================
  Files          90       90              
  Lines        8720     8720              
==========================================
- Hits         5133     5122      -11     
- Misses       3000     3011      +11     
  Partials      587      587

Flag	Coverage Δ
combined	`58.73% <ø> (-0.13%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

alpe · 2025-12-15T15:02:30Z

The changes look good. Please don't merge now to avoid conflicts

tac0turtle · 2025-12-15T15:45:46Z

The changes look good. Please don't merge now to avoid conflicts

yes will hold off, this isnt high priority

* main: (34 commits) feat: make reaper poll duration configurable (#2951) chore!: move sequencers to pkg (#2931) feat: Ensure Header integrity on DA (#2948) feat(testda): add header support with GetHeaderByHeight method (#2946) chore: improve code comments clarity (#2947) chore(sequencers): optimize store check (#2945) fix: make evm_execution more robust (#2942) fix(sequencers/single): deterministic queue (#2938) fix(block): fix init logic sequencer for da epoch fetching (#2926) feat: use DA timestamp (#2939) chore: improve code comments clarity (#2943) build(deps): bump libp2p (#2937) build(deps): Bump actions/cache from 4 to 5 (#2934) build(deps): Bump actions/download-artifact from 6 to 7 (#2933) build(deps): Bump actions/upload-artifact from 5 to 6 (#2932) feat: DA Client remove interface part 3, replace types with new code (#2910) DA Client remove interface: Part 2.5, create e2e test to validate that a blob is posted in DA layer. (#2920) fix(syncing): skip forced txs checks for p2p blocks (#2922) build(deps): Bump the all-go group across 5 directories with 5 updates (#2919) chore: loosen syncer state check (#2927) ...

move seqeucners to pkg

69e52f6

github-actions bot assigned tac0turtle Dec 15, 2025

gemini-code-assist bot reviewed Dec 15, 2025

View reviewed changes

tac0turtle added 4 commits December 15, 2025 15:23

Merge branch 'main' into marko/seqeucners_pkg

3050f0e

Merge branch 'main' into marko/seqeucners_pkg

2079a1a

Merge branch 'main' into marko/seqeucners_pkg

fbd94ce

lint

d70f76e

tac0turtle marked this pull request as ready for review January 3, 2026 15:44

julienrbrt approved these changes Jan 4, 2026

View reviewed changes

tac0turtle merged commit 66b491b into main Jan 5, 2026
28 of 30 checks passed

tac0turtle deleted the marko/seqeucners_pkg branch January 5, 2026 11:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore!: move sequencers to pkg #2931

chore!: move sequencers to pkg #2931

Uh oh!

tac0turtle commented Dec 15, 2025

Uh oh!

github-actions bot commented Dec 15, 2025 •

edited

Loading

Uh oh!

claude bot commented Dec 15, 2025 •

edited

Loading

Uh oh!

gemini-code-assist bot commented Dec 15, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Dec 15, 2025

Uh oh!

codecov bot commented Dec 15, 2025 •

edited

Loading

Uh oh!

alpe commented Dec 15, 2025

Uh oh!

tac0turtle commented Dec 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	- Execution (`core/execution/execution.go`) - State transition interface and types
	- Executor (`core/execution/execution.go`) - State transition interface and types

chore!: move sequencers to pkg #2931

chore!: move sequencers to pkg #2931

Uh oh!

Conversation

tac0turtle commented Dec 15, 2025

Overview

Uh oh!

github-actions bot commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Complete

Tasks

Summary

Detailed Findings

✅ What's Good

⚠️ Issues Found

Code Quality Assessment

Architecture & Design

Error Handling & Resilience

Security

Performance

Testing

Documentation

Recommendations

Must Fix Before Merge

Good to Have

Notes

Conclusion

Uh oh!

gemini-code-assist bot commented Dec 15, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

alpe commented Dec 15, 2025

Uh oh!

tac0turtle commented Dec 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Dec 15, 2025 •

edited

Loading

claude bot commented Dec 15, 2025 •

edited

Loading

codecov bot commented Dec 15, 2025 •

edited

Loading