[abdulmajidcse]

Pull Request Checklist

Q	A
Branch?	4.x
Bug fix?	no
New feature?	yes
Deprecations?	no
Issues	#359
License	MIT

Problem - Solution of this issue

Doppar's #[Temporal] models already snapshot every create, update, delete and restore into an append-only history table, and support point-in-time reads through Model::at($datetime). There was no way to keep a proposed change to a record, review it against the live state, and then apply or discard it without touching the live history.

What this adds

A model opts in with #[Temporal(branches: true)]. A named branch is forked from the main timeline, takes further proposed versions, and can be compared with the live record and restored from:

#[Temporal(branches: true)]
class Invoice extends Model { /* ... */ }

$invoice->branch('proposed', now()->subDay());   // fork from main

$invoice->total = 150;
$invoice->saveToBranch('proposed');              // live row untouched, model stays dirty

$invoice->diffAgainstBranch('proposed');         // main vs proposal
Invoice::at(now(), 'proposed')->find($invoice->id);
$invoice->history('proposed');

$invoice->restoreTo(now(), 'proposed');          // make the proposal live
$invoice->restoreVersion($historyId);            // or restore one exact entry

• Model::at(), history(), diff(), rewindTo() and restoreTo() take an optional branch, defaulting to main. history(null) returns every branch.
• branch($name, $fromDatetime) forks a branch from the main snapshot at that time. Forking an existing name throws. The branch timeline starts when the fork is created.
• saveToBranch($name) appends the in-memory state to an existing branch without writing the live row. The model stays dirty, so a later save() still makes it live.
• diffAgainstBranch($name) returns at, main, branch {name, snapshot}, changes, added and removed. It returns null when the live record is deleted.
• restoreVersion($historyId) restores one history entry. It returns false for an unknown id and throws for a deleted entry.
• Branch names are 1-50 characters of [a-z0-9_-], validated on every entry point. main is reserved. Names are lowercase-only so that MySQL's case-insensitive collation and PostgreSQL/SQLite cannot disagree about which branch a name refers to.
• Any branch other than main on a model without branches: true throws a RuntimeException. On a branchable model whose history table has not been migrated yet, writes to main still succeed and are recorded. Every branch operation throws and asks you to run migrate:temporal.
• diff() now runs both snapshots through the model's casts and hides unexposable attributes, then compares them with upstream's snapshotValuesDiffer(), so 5 and '5' are equal and null and '' differ. The result also includes a branch key.

Behaviour of existing temporal models

Models without branches: true behave as before, apart from two additive changes:

• diff() results gain a branch key (always main), and values are cast and hidden attributes filtered out before comparing.
• restoreVersion() is new. It refuses deleted entries on every temporal model.

rewindTo() and restoreTo() follow upstream's rule from b3fa670 on every model: at a point where the record was deleted, rewindTo() returns null and restoreTo() returns false. This branch is merged with the current 4.x.

Schema changes

An opt-in branch column (VARCHAR(50) / TEXT, NOT NULL DEFAULT 'main') and a (record_id, branch, valid_from) index, generated per driver in TemporalManager the same way as the opt-in actor column. migrate:temporal adds the column and the index to existing history tables, reports them separately, and clears the column cache. Rows written before the migration take the default main.

Column-existence checks are cached per PDO connection (a WeakMap), including negative results, and the cache is cleared by TemporalManager::resetCache() and by migrate:temporal.

Tests

• tests/Model/TemporalBranch/ runs the same behaviour suite on SQLite, MySQL and PostgreSQL (TemporalBranchSqliteTest, TemporalBranchMysqlTest, TemporalBranchPgsqlTest). It covers forking, branch writes, branch diffs, restore guards, name validation including uppercase names, writes before migration, uncast type equivalence, and same-microsecond ordering.
• tests/Console/MigrateTemporalCommandTest.php covers fresh tables, adding the column and index to existing tables, idempotent reruns and dry-run output.
• Ran against MySQL 8.4 and PostgreSQL 16 with no skipped branch tests. Full suite after merging 4.x: 4593 tests, 0 failures. PHPStan clean.

Docs

temporal-orm.md documents branches: true, the new methods, branch naming rules and the restore guards: doppar/docs#116

Checklist

• Tests have been added or updated
• Documentation has been updated if necessary
• Code follows the project coding standards
• All tests pass locally

[techmahedy]

Thanks for the PR. The design is sound and opt-in, and it leaves existing temporal models untouched. The 17 tests in TemporalBranchTest and MigrateTemporalCommandTest pass on SQLite. A few changes are needed before merge to be honest.

Thanks for the PR. The design is sound and opt-in, and it leaves existing temporal models untouched. The 17 tests in TemporalBranchTest and MigrateTemporalCommandTest pass on SQLite. A few changes are needed before merge.

Must fix

1. diff() still compares values with (string)

File: src/Phaseolies/Database/Temporal/InteractsWithTemporal.php (diff() loop)

After castForGet, array/json casts become PHP arrays.

• (string) $array gives "Array" plus a warning, so two different arrays compare equal and the change is silently dropped.
• Objects without __toString throw an Error.
• false, null and '' all become '', so they compare equal.

Change: compare with ==, or json_encode both sides for arrays and objects.
Test to add: a model with an array/json cast and a bool cast, where a change in each shows up in changes.

2. Missing branch column on a branchable model returns wrong data

Files: TemporalBuilder.php, InteractsWithTemporal.php::snapshotAt(), TemporalManager.php::snapshotBranch()

• When the column is missing, the branch filter is skipped. Invoice::at($t, 'proposed') then returns main-timeline rows as if they were the branch.
• snapshotBranch() has no column guard, unlike snapshot(). $model->branch() on an unmigrated table fails with a raw PDOException.

Change: if the model is branchable and the column is missing, throw a RuntimeException that says to run migrate:temporal.
Test to add: a branchable model on a table without the column, covering at(), branch() and history().

3. Branches can't be written to, and restoreVersion() is missing

Issue #359 asks for "proposed state … safely reviewable" and a guarded restoreVersion().

• branch() creates one frozen copy. Nothing writes further versions to a branch. The $branch parameter on TemporalManager::snapshot() has no caller.
• There is no way to diff main against a branch. diff() only compares two times on one branch.
• restoreVersion($historyId) doesn't exist. restoreTo(datetime) has no guards, such as refusing to restore a deleted row.

Change: either implement these, or describe the PR as partial and don't use "Fixes #359".

Should fix

4. ALTER TABLE migration path doesn't create the index

File: MigrateTemporalCommand.php

Fresh tables get (record_id, branch) and migrated tables don't. Add a CREATE INDEX after addBranchColumnSql() for MySQL, PostgreSQL and SQLite.

5. Branch name validation

• The column is VARCHAR(50). Strict MySQL and PostgreSQL reject longer names and SQLite accepts them, so behaviour differs by driver.
• Validate length and charset, and call the check from at(), diff(), rewindTo(), restoreTo() and history() as well as branch().
• Calling branch() twice with the same name appends another branched row and quietly overwrites the first fork. Decide whether to throw or document it.

6. Fork timestamp semantics

• The fork row's valid_from is now, not the source point. at($earlier, 'proposed') returns null. Document this, or store the source time.
• Add history_id as a tiebreak in ORDER BY valid_from DESC so same-microsecond rows resolve predictably.

7. Run the tests on MySQL and PostgreSQL in CI

The new tests run on SQLite only. Port TemporalBranchTest to the driver-trait layout used by tests/Model/SoftDelete/SoftDeleteBehavior.php (Mysql, Pgsql and Sqlite subclasses). Manual runs reported in the PR description won't catch regressions.

Minor

• phpstan baseline. Don't suppress the findings.
  • Replace $attr?->branches ?? false with a non-nullsafe form.
  • Declare $__branch on the model.
• Column cache. hasColumn caches false for the process lifetime, so long-running workers won't see a newly migrated column until restart. Consider not caching negatives.
• PostgreSQL column check. The information_schema query has no table_schema filter.
• visibleSnapshot(). It builds new static() on every call. Build it once per diff().

[abdulmajidcse]

@techmahedy, updated again.

[techmahedy]

@abdulmajidcse # Review, round 2: Branchable temporal records (431b52c)

Thanks for the quick follow-up. Most of the first round is properly fixed, and the new saveToBranch(), diffAgainstBranch() and restoreVersion() close the biggest gap. I merged the branch and ran it: the full suite passes (4518 tests, 0 failures, 858 skipped because no MySQL or PostgreSQL server is available here) and PHPStan is clean.

Reading the new code and probing it against SQLite turned up two regressions and a few smaller problems. Each item below was reproduced by running it, except the MySQL collation item, which is reasoned from the DDL and needs a MySQL run to confirm.

Status of round one

#	Item	Status
1	diff() compared with (string)	Fixed for arrays, bools and null. Introduced a new false positive, see 2 below
2	Missing branch column returned main rows	Fixed for reads. The write path now has a worse problem, see 1 below
3	No branch writes, no restoreVersion()	Added. See 3 and 4 below for rough edges
4	ALTER path did not create the index	Fixed
5	Branch name validation	Fixed (1-50 chars, [A-Za-z0-9_-], on every entry point, duplicates throw). Case sensitivity is open, see 6
6	Fork timestamp semantics	Documented in the docblock
6b	history_id tiebreak	Fixed in snapshotAt, history, find and fetchTemporalRows
7	Tests on all three drivers	Ported to the Sqlite/Mysql/Pgsql layout
-	phpstan baseline suppressions	Removed
-	PostgreSQL column check without schema	Fixed in hasColumn and the migrate command
-	new static() on every diff()	Fixed, built once

Must fix

1. Writes on an unmigrated branchable table now save the record and then throw

File: TemporalManager::snapshot()

snapshot() now calls assertBranchColumn() for every branches: true model. That runs in the after_created, after_updated and after_deleted hooks, which fire after the live write.

Reproduced: add branches: true to a model whose history table has no branch column, then Model::create([...]):

create on unmigrated table -> thrown='Temporal branch column is missing from branchable_records_history; run migrate:temporal.'
live_rows=1  history_rows=0

The live row is saved, no history row is written, and the caller gets an exception. A caller that retries creates duplicates. Deploying the attribute before running migrate:temporal therefore breaks every write on that model.

The previous behavior was safe: skip the branch value when the column is absent. Rows written before the migration take the column default 'main' once it is added, so they end up correct.

Change: keep the guard on reads and on branch operations (at, history, diff, branch, saveToBranch, restoreTo, restoreVersion), but let snapshot() for 'main' skip the branch key when the column is missing. Only throw from snapshot() when a named branch is being written.
Test to add: create, update and delete a branchable record against a branch-less history table and assert the live write succeeds and nothing throws.

2. diff() now reports a change when only the PHP type of an uncast column differs

File: InteractsWithTemporal.php, sameCastValue()

=== is right for cast values, but uncast columns hold whatever type the driver or the form supplied. Reproduced with two snapshots whose status is 5 and '5':

diff of status 5 vs '5' -> changes={"status":{"from":5,"to":"5"}}

Before this change that was correctly treated as no change. Snapshots mix types in real use: a created snapshot comes from the values you assigned, an updated one from the loaded row, and MySQL returns strings or ints depending on PDO::ATTR_EMULATE_PREPARES. Users would see phantom changes on every numeric column.

Change: keep strict comparison for columns that have a cast and for arrays and objects. For uncast scalars, compare as strings, except null against '', which should still differ.
Test to add: an uncast column that is 5 in one snapshot and '5' in the other must not appear in changes.

Should fix

3. After saveToBranch() the model claims to be saved and save() does nothing

File: saveToBranch()

It ends with $this->setOriginalAttributes($this->getAttributes()), which marks the proposed values as clean. Reproduced:

after saveToBranch: dirty=[]  save()=true  live price in DB=100  in-memory price=150

The object holds 150, reports no pending changes, and save() returns true without writing anything. Anyone who calls save() next, expecting the proposal to go live, silently loses it, and any code that reads the in-memory model sees a value that was never saved.

Change: do not reset the original attributes, so the model stays dirty and an explicit save() still means "make this live". If you want a clean scratch object, return a copy instead. Document which one it is.

4. restoreTo() and restoreVersion() change behavior for models that do not use branches

File: restoreTo(), assertNotDeletedAt()

restoreTo() on a non-branchable model whose record was deleted used to restore the pre-delete snapshot. It now throws:

restoreTo(after delete) on NON-branchable model -> thrown='RuntimeException: Cannot restore a deleted temporal version.'
rewindTo for the same time still returns a model

The guard is what the issue asked for, so the change is fine, but it is a change for every temporal model. The PR description says models that do not opt in are "completely unaffected", and the docs (temporal-orm.md) say restoreTo() returns true or false with no mention of an exception.

Change: update the PR description, and add the exception to the "Restoring a Record" docs and an upgrade note. Consider whether rewindTo() should apply the same rule, since rewindTo() and restoreTo() now disagree about the same instant.

5. Missing columns are never cached any more

File: TemporalManager::hasColumn()

Only positive answers are cached. Reproduced: five calls to hasActorColumn() on a table without an actor column run five metadata queries. A trackActor: true model whose table has no actor column (the "resilient actor detection" case) now pays an extra PRAGMA/information_schema/SHOW COLUMNS query on every snapshot.

Also the cache key uses spl_object_id($pdo). Object ids are reused once a connection is freed, so a new connection can inherit a stale true from an old one.

Change: cache negatives for a short time, or per connection name instead of object id, and clear them in migrate:temporal and resetCache().

6. Branch names are case sensitive in PHP but not necessarily in the database

Files: validateBranchName(), branch(), the MySQL DDL

'main' is rejected with ===, but 'Main', 'MAIN' and 'Draft'/'draft' pass validation. The MySQL history table is created with COLLATE=utf8mb4_unicode_ci, which is case-insensitive, so WHERE branch = 'MAIN' matches the main rows. That means Model::at($t, 'MAIN') can silently read the main timeline, and Draft and draft are the same branch on MySQL but two branches on PostgreSQL and SQLite. I have not run this against MySQL here, please confirm.

Change: normalize names to lowercase on every entry point and reject main after normalizing, or create the branch column with a binary collation.

7. diffAgainstBranch() result shape

It returns 'branch' => 'main → proposed' and the same timestamp for both from and to. That reuses the diff() shape awkwardly. Return a dedicated shape, for example main, branch and at. It also compares against the latest non-deleted main snapshot, so for a record that has since been deleted it quietly diffs against stale data. Return null or throw when the live record is deleted.

Minor

• migrate:temporal prints branch, branch index columns added. The wording is wrong when only the index is added (branch index column added). Report columns and indexes separately.
• restoreVersion() selects branch and never uses it. Either use it, for example to refuse a version from a branch that does not exist, or drop it from the SELECT.
• restoreVersion() returns false for an unknown history id and throws for a deleted one. Pick one convention and document it.
• fetchTemporalRows() gained a second correlated subquery per row for the tiebreak. It is correct, but check the plan on a large history table: the (record_id, branch) index does not cover valid_from. Consider (record_id, branch, valid_from).
• Add saveToBranch(), diffAgainstBranch(), restoreVersion() and the branches: true option to temporal-orm.md. The docs currently say nothing about branches.

Checklist before re-review

• Writes on an unmigrated branchable table succeed and do not throw (1), with a test
• Uncast 5 versus '5' is not reported as a change (2), with a test
• saveToBranch() leaves the model dirty (3), with a test
• Behavior change of restoreTo() on non-branchable models documented and in the PR description (4)
• Negative column results cached per connection (5)
• Branch names normalized, or a case-sensitive column (6), confirmed on MySQL
• Docs for the new methods

[abdulmajidcse]

@techmahedy, check now, please.

[techmahedy]

@techmahedy, check now, please.

thanks for update, perfect now, make sure your all the functions follow this doc comments convention
https://doppar.com/versions/4.x/contributions#phpdoc

[techmahedy]

https://doppar.com/versions/4.x/contributions#phpdoc
follow this convention please

[techmahedy]

https://doppar.com/versions/4.x/contributions#phpdoc

[techmahedy]

https://doppar.com/versions/4.x/contributions#phpdoc

[techmahedy]

https://doppar.com/versions/4.x/contributions#phpdoc