# bstore-java Code Review

Review date: 2026-03-17

This review focuses on correctness, architectural fit to the current goals, and
remaining risks before using `bstore-java` as the reference implementation for
other language ports.

## Findings

No new correctness findings were discovered in this final pass after the latest
meta-history fixes.

## Non-Findings

These areas looked acceptable for the current scope:

- `dbo` primary-key semantics are now explicit and much less error-prone.
- SQL `IN` handling is parameterized.
- `Action.first()` cleanup is fixed.
- `SQLReader.hasNext()` no longer advances repeatedly.
- `dbo.meta` is now appropriately lean for the current goals.
- `ChangeEvent` as canonical history and `DataOriginator` as module registry is a
  coherent split.
- `terminal.save(changeEvent)` now persists the event row without implicitly
  applying schema changes.
- `apply_changes(...)` now logs pending events before attempting structural
  changes, so failed batches do not lose the canonical change record.
- `upgradeMetaSchema(...)` now respects the caller-supplied `originatorId` and
  `migrationId`.

## Current Limits That Seem Acceptable

These are limitations, but they appear aligned with the current requirements:

- field rename is treated as drop + add rather than a first-class rename
- schema moves are rejected explicitly
- PK/auto-increment/unique/index rewrites are not yet supported in `apply_changes(...)`
- downgrade support is schema-shape oriented, not a full data-preserving rollback engine
- migration batches are still best-effort rather than transactional across all
  DDL plus history writes; the log is preserved, but partial application can
  still occur on backends or change sets that are not fully transactional

## Suggested TODO List

Priority 1

- Decide whether to add optional transactional migration batches on backends
  that support transactional DDL, or keep the current best-effort semantics as
  the long-term contract.
- Add a short public example showing when to use `terminal.save(changeEvent)`
  versus `meta.apply_changes(...)`.

Priority 2

- If `ChangeEventHero` is no longer part of the intended public design, remove it
  or explicitly deprecate it to avoid confusion.
- Consider exposing an explicit "planned but unapplied" query path for pending
  changes now that unapplied events are preserved in the log.

Priority 3

- Review old exploratory tests and either remove them permanently or mine any
  remaining useful cases into the curated suites.
- Prepare a concise cross-language contract document derived from the current
  Java `dbo` and `meta` behavior before porting further.

## Bottom Line

The project is in a strong reference-implementation state for the original goals.
The remaining important risks are mostly around future migration ergonomics and
transaction semantics, not everyday CRUD correctness or core meta design.