Skip to content

Conversation

@gfyrag
Copy link
Contributor

@gfyrag gfyrag commented Dec 18, 2025

No description provided.

@gfyrag gfyrag requested a review from a team as a code owner December 18, 2025 13:29
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 18, 2025

Caution

Review failed

Failed to post review comments

Walkthrough

This PR introduces architectural improvements spanning infrastructure, API, and storage layers. Key changes include restructuring Pulumi deployment components into a modular pattern, refactoring transaction/log ID handling from integers to pointers, extending ledger controller interfaces with transaction locking and metadata timestamping, implementing a factory-based database abstraction, unifying API error handling, and adding ledger state tracking during initialization. Database migrations add new SQL functions for hashing and state management, while API routing now accepts explicit versioning.

Changes

Cohort / File(s) Summary
Build & Configuration
.gitignore Earthfile deployments/pulumi/Earthfile
Updated Earthly base image to v0.19.1, added Pulumi config files to gitignore, and enhanced Earthfile to download Speakeasy binary dynamically and merge OpenAPI overlays for SDK generation.
Documentation
README.md docs/api/README.md pkg/client/*
Updated community links from Slack to GitHub Discussions, added V2ImportLogsRequest to API schema with idempotency-key handling, expanded SDK documentation with environment-based credentials and v1/v2 endpoint clarification.
Pulumi Deployment Infrastructure
deployments/pulumi/main.go deployments/pulumi/pkg/api/* deployments/pulumi/pkg/storage/* deployments/pulumi/pkg/common/* deployments/pulumi/pkg/devbox/* deployments/pulumi/pkg/generator/* deployments/pulumi/pkg/monitoring/* deployments/pulumi/pkg/provision/* deployments/pulumi/pkg/utils/* deployments/pulumi/pkg/config/* deployments/pulumi/tools/*
Introduced modular component-based deployment architecture with separate components for API, Storage, Generator, Provision, and DevBox; added comprehensive configuration layer supporting ingress, RDS/Postgres, OTLP/Jaeger monitoring; implemented image configuration helpers and JSON schema generation tooling.
Core API & Routing
internal/api/module.go internal/api/router.go internal/api/common/errors.go internal/api/common/middleware_resolver.go cmd/serve.go cmd/root.go internal/api/v1/routes.go internal/api/v1/controllers_config.go internal/api/v2/routes.go
Added dynamic version parameter to router, unified error handling via HandleCommonWriteErrors and InternalServerError, exported GetInfo handler, added panic recovery middleware with OTLP integration, and restructured OTLP flag wiring.
Handler & Controller Updates
internal/api/bulking/* internal/api/v1/controllers_*.go internal/api/v2/controllers_*.go
Introduced JSON streaming bulk handler alongside text streaming, renamed ScriptStreamBulkHandler to TextStreamBulkHandler, updated error handlers from HandleCommonErrors to HandleCommonWriteErrors, and adjusted mock signatures.
Transaction & Log Model
internal/log.go internal/transaction.go internal/metadata.go
Changed Log/Transaction ID from int to *int with new WithID() builder methods, added metadata composition helpers (SpecMetadata, MarkReverts, RevertMetadata, ComputeMetadata), and adjusted hashing to ignore actual ID values.
Storage Layer Interfaces
internal/storage/bucket/bucket.go internal/storage/bucket/default_bucket.go internal/storage/system/store.go internal/storage/system/factory.go internal/controller/ledger/store.go
Refactored Bucket interface to accept bun.IDB parameter per-call instead of storing DB, introduced GetLastVersion() method, added factory pattern via StoreFactory, updated Store interface with LockLedger() returning DB handle and cleanup function, and extended metadata methods with timestamps.
Ledger Controller & State Tracking
internal/controller/ledger/controller.go internal/controller/ledger/controller_default.go internal/controller/ledger/controller_*.go internal/controller/system/state_tracker.go
Extended BeginTX() to return *bun.Tx, added LockLedger() with bun.IDB handle and cleanup function, introduced state-tracking facade for synchronized ledger initialization, added import logic with per-log transactions and concurrent-transaction handling.
Storage Driver
internal/storage/driver/driver.go internal/storage/driver/module.go internal/storage/bucket/migrations.go
Refactored Driver to use factory pattern for store/bucket creation, added tracer integration, implemented per-transaction ledger creation with conflict handling, changed bucket API to require bun.IDB parameter, and templated migrations with transactional awareness.
Database Migrations
internal/storage/bucket/migrations/*
Added new state column to ledgers, introduced compute_hash() and set_log_hash() PostgreSQL functions, implemented batch-based migration workflows for PCV/memento fixes and metadata reconciliation, added transactional index creation conditionals, and filled timestamp/sequence fields across transactions and logs.
Ledger Store Operations
internal/storage/ledger/store.go internal/storage/ledger/transactions.go internal/storage/ledger/logs.go internal/storage/ledger/balances.go internal/storage/ledger/accounts.go
Updated BeginTX() to return transaction handle, added LockLedger() implementation, refactored balance queries with migration-aware paths (upgrading vs. post-upgrade), introduced scoped-select helpers, extended metadata operations with timestamps, and updated account metadata upsert merge order.
Query & Pagination Helpers
internal/storage/ledger/paginator_*.go internal/storage/ledger/legacy/utils.go internal/storage/ledger/resource.go
Exported pagination helpers (FindPaginationFieldPath, FindPaginationField, EncodeCursor), added support for custom pagination columns via SQL-type conversion, refactored legacy pagination to thread filters through, changed filter storage from single-value to multi-value maps (map[string][]any).
Resource Builders
internal/storage/ledger/resource_*.go
Replaced direct DB queries with scoped-select builders, removed explicit ledger filtering conditions, added effective_date-based ordering, included account_array in address segmentation, and updated volume/balance aggregations to reference public schema functions.
Ledger Controller Implementations
internal/controller/ledger/errors.go internal/controller/ledger/log_process.go
Exported NewErrImport() constructor, added ErrConcurrentTransaction error type with factory function, adjusted log insertion to dereference pointer IDs, and updated idempotency hash checks.
Upgrade & Command Wiring
cmd/buckets_upgrade.go
Replaced direct storage/DB handling with FX-based dependency injection, integrated OTLP tracing and logging via FX modules, introduced withStorageDriver wrapper for controlled app startup/shutdown.
Test Infrastructure
internal/storage/bucket/bucket_generated_test.go internal/controller/ledger/*_test.go internal/api/*_test.go internal/storage/ledger/*_test.go
Generated gomock mocks with updated signatures (bun.Tx, bun.IDB, timestamp parameters), added TestMain database setup for integration tests, updated mock expectations to return pointer IDs and include transaction handles, extended test coverage with concurrency and adapter interoperability checks.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Areas requiring extra attention:

  • Pulumi deployment restructuring: Verify all component dependencies (namespace, API, Storage, Provision, Generator, DevBox) initialize in correct order with proper error propagation and output registration.
  • Transaction/Log ID pointer changes: Confirm all ID dereferencing is consistent; check hash computation logic ignores actual ID; validate cursor/pagination handling with pointer IDs.
  • Storage layer factory pattern refactoring: Review bun.IDB parameter threading through all bucket/store operations; verify DB lifecycle management and cleanup functions in LockLedger.
  • State tracker implementation: Ensure ledger state transitions (initializing → in-use) are atomic and respect transaction boundaries; verify no race conditions around state mutations.
  • Database migrations with transactional awareness: Test conditional CONCURRENTLY logic; validate batch-processing loops with proper offset tracking and notifications; check migration ordering and idempotency.
  • API error handling unification: Verify HandleCommonWriteErrors and InternalServerError properly distinguish error types (validation, not-found, conflict) and propagate OTLP context.
  • Cross-layer signature changes: Ensure all call sites of modified interfaces (BeginTX, LockLedger, CommitTransaction, etc.) are updated consistently; check test mocks align with production code.

Possibly related PRs

  • fix: import from 2.1 #689: Implements same core ID changes (Log/Transaction → *int, WithID methods, GetInfo export, NewRouter versioning) and related internal API/storage refactoring.
  • feat: refine pulumi #721: Modifies Pulumi deployment code with similar component wiring and configuration restructuring across deployments/pulumi/pkg/*.
  • fix: import regression #775: Overlaps on ledger state constants, ID pointer handling, and transaction interface changes (BeginTX returning *bun.Tx).

Suggested reviewers

  • paul-nicolas
  • laouji
  • flemzord

Poem

🐰 Hoppy infrastructure hops about,
Pointers now store IDs, no doubt!
Pulumi components dance so neat,
State tracking keeps the ledger sweet. 🌿
Transactions lock with graceful care,
Where hashing functions float through air.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.56% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: pit regression' is clear and directly describes the main issue being addressed—a regression related to point-in-time (PIT) functionality.
Description check ✅ Passed Although the PR description field is empty, the PR objectives provide comprehensive context about the changes, fixes, and features included in this pull request.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch hotfix/v2.2/pit-regression

Comment @coderabbitai help to get the list of available commands and usage tips.

@gfyrag gfyrag changed the base branch from main to release/v2.2 December 18, 2025 13:32
@gfyrag gfyrag merged commit 0972469 into release/v2.2 Dec 18, 2025
1 check passed
@gfyrag gfyrag deleted the hotfix/v2.2/pit-regression branch December 18, 2025 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants