feat(ordering): add getNextPositionForUpdate for stale-snapshot-safe position allocation

[megyptm]

Closes #31

الملخص

بعد المراجعة المعمارية، تحتفظ getNextPosition() بسلوكها المنشور في v1.4.0 (قراءة MAX() عادية، بلا قفل، بلا معاملة). وهذا يُبقي سطر PACKAGE_BUILDING_STANDARD.md الذي ينص على أنها «لا تبدأ معاملة ولا تأخذ قفلًا» صحيحًا. إصلاح الـ snapshot القديم دالة جديدة إضافية، ويبقى المساران مدعومين جنبًا إلى جنب (لا @deprecated على getNextPosition()، بقرار المالك: للقراءة بلا قفل استخدامات مشروعة طويلة الأمد). وثّقت PHPDoc وREADME والمرجع متى تُستخدم كل دالة، وبقاعدة الـ snapshot الدقيقة للقراءة العادية: تحت REPEATABLE READ لا تُحدِّث قراءات القفل الـ snapshot، فلا يجعل القفل بعد وجوده القراءة العادية حديثة، وإعادة المحاولة في المعاملة نفسها (مثل بعد خطأ مفتاح مكرر) تُرجع القيمة القديمة نفسها:

ScopedOrderingManager::getNextPositionForUpdate(PDO, ScopedOrderingConfig, int|string|null $scopeValue = null): int

• تشترط معاملة PDO نشطة على الاتصال، وإلا ترمي InvalidOrderingOperationException الموجود (موثّق الآن بأنه يغطي الوسائط غير الصالحة أو شرطًا مسبقًا غير مستوفى). لا تبدأ معاملة ولا تنهيها؛ يبقى القفل حتى ينهي المستدعي معاملته، لذا يجب الإدراج على الاتصال والمعاملة نفسيهما. تتركّب مع PdoTransactionRunner / PdoSavepointTransactionRunner دون Runner جديد.
• تقرأ SELECT <order> ... ORDER BY <order> DESC, <id> DESC LIMIT 1 FOR UPDATE. قراءة القفل تعيد آخر حالة مُثبَّتة بدل snapshot الـ REPEATABLE READ، فلا يضيع صف ثبّته اتصال آخر بعد إنشاء الـ snapshot (سبب تكرار display_order في getNextPosition() reads a stale MAX inside a caller transaction with an older snapshot, producing duplicate positions #31).
• تُكرَّر القراءة القافلة حتى تُرجع قراءتان متتاليتان قيمة الترتيب نفسها (قراءتان قافلتان على الأقل لكل استدعاء، حتى للنطاق الفارغ). السبب: قراءة انتظرت قفل الذيل السابق قد تستأنف بعد أن ثبّتت معاملة أخرى ذيلًا أعلى، فتُرجع الذيل القديم + 1، أي موضعًا مكررًا؛ أُعيد إنتاج ذلك على MySQL 8.4.10 تحت READ COMMITTED (نتيجة 3 بدل 4). التكرار لا يستمر إلا ما دام الذيل يتغيّر، وكل قراءة مكررة قد تنتظر قفلًا أو تفشل بمهلة قفل أو deadlock.
• نفس التحقق من النطاق ومعالجة الحذف المنطقي والنطاق القابل لـ NULL الخاصة بـ getNextPosition() (مُنشئ WHERE مشترك؛ SQL الدالة القديمة لم يتغير).

لم يتغير أي توقيع أو استثناء أو ملكية معاملات قائمة. التغيير العام الوحيد هو إضافة الدالة (إضافة بمستوى minor؛ تصنيف الإصدار قرار المالك). تحقق من ذلك بمقارنة الواجهة العامة لـ v1.4.0 بالفرع بالـ reflection (الفرق الوحيد دالة مضافة) وبتشغيل اختبارات v1.4.0 نفسها على src/ الخاص بالفرع.

لماذا هذا الاستعلام

قياس على MySQL 8.0.46 و8.4.11 حقيقيين بنطاقات من 1000 صف (performance_schema.data_locks وEXPLAIN):

الحالة	الأقفال
MAX() ... FOR UPDATE مع فهرس (scope, order)	كل صفوف النطاق مع الفجوة التالية (مرفوض)
ORDER BY DESC LIMIT 1 FOR UPDATE مع فهرس (scope, order) وذيل حي	آخر صف حي مع الفجوة فوقه (أول سجل في النطاق التالي تحت REPEATABLE READ)
الحالة نفسها وأعلى 995 صفًا محذوفة منطقيًا	نحو 997 سجل فهرس و996 سجل PK
بلا فهرس (scope, order) صالح (جدول الاختبار العام لا يملكه)	كل صفوف الجدول وفجواته
READ COMMITTED	أقفال سجلات فقط بلا قفل فجوة

رُفضت FOR SHARE لأن مستدعيَين يمسكان قفلًا مشتركًا ثم يتعارضان عند الإدراج.

الضمان والحدود (موثقة في الكود وREADME والمرجع)

المضمون: النتيجة تعكس آخر حالة مُثبَّتة؛ وعلى نطاق غير فارغ يتسلسل المستدعون الذين يستخدمون قراءات قفل أيضًا (getNextPositionForUpdate() وmoveWithinScope()) على آخر صف حي. أُثبت ذلك باختبارات MySQL حقيقية في الاتجاهين.

غير مضمون: النطاق الفارغ (لا صف لقفله، فعلى المستدعي قفل صف أب أو استخدام قيد UNIQUE)؛ الكاتبون بلا قراءات قفل (إلا فجوة REPEATABLE READ)؛ حجم الأقفال يعتمد على الفهرس والصفوف المحذوفة منطقيًا؛ قد تلمس الفجوة حدود نطاق آخر؛ تنتشر استثناءات PDOException لمهلة القفل والـ deadlock كما هي وإعادة المحاولة مسؤولية المستدعي (انظر #32، لم يُلمس). تحت REPEATABLE READ يفشل عادةً استدعاء ينتظر خلف معاملة تُدرج بعدها فوق الصف المقفول بـ deadlock بدل أن يستأنف.

الاختبارات

ScopedOrderingManagerStaleSnapshotTest (25 اختبارًا على MySQL حقيقي):

• الـ snapshot القديم للنطاقات المحددة والعامة والقابلة لـ NULL، والنطاق الفارغ، والصفوف المحذوفة منطقيًا، واستقلال النطاقات، والإدراج ثم moveWithinScope() بمواضع فريدة متصلة.
• الرفض خارج معاملة، وبقاء ملكية المعاملة، والتحقق من النطاق، وحجب كل من الدالة وmoveWithinScope() للآخر، وعدم حجب صفوف النطاقات الأخرى، وحد النطاق الفارغ.
• متى تكون القراءة العادية آمنة: تبقى قديمة (ويُعيد تكرارها القيمة نفسها) إذا قفل المستدعي بعد أن وُجد الـ snapshot، وتكون حديثة إذا سبق قفل التسلسل أول قراءة غير قافلة.
• اختبارات التزامن بعملية PHP فرعية على اتصال مستقل: مخصِّص ثانٍ يُنتظر خلف قفل الذيل، ثم تُدرج المعاملة الأولى ذيلًا أعلى وتُثبّته أثناء انتظاره.
  • READ COMMITTED: يجب أن يرى المنتظر الذيل الجديد ويُرجع 4 والحالة المثبّتة [1, 2, 3].
  • REPEATABLE READ (حالتان: منتظر خفيف ومنتظر ثقيل يرجّح أن تكون المعاملة المُدرِجة ضحية الـ deadlock): يُقرأ الحالة المثبّتة من اتصال آخر ويُتحقق أنها تطابق النتيجة (الصف المُدرَج موجود فقط إن ثُبّتت معاملته، ولا موضع مشغول مرتين)، وأن الموضع الذي يُبلِغ عنه المنتظر هو أول موضع حر تمامًا، أو أنه فشل بـ deadlock 1213 والإدراج مثبّت.
  • قفل الذيل المتراكم: بعد تكرار القراءة يبقى الذيلان (القديم والجديد) مقفولين حتى تنتهي المعاملة، ولا يُقفل الصف الأقدم.
• الحراس: getNextPosition() القديمة لا تأخذ قفلًا حتى داخل معاملة ولا ينتظرها قفل اتصال آخر.
• OrderingPublicApiRegressionTest يثبّت التوقيع الجديد وأن أيًّا من getNextPosition() وgetNextPositionForUpdate() لا يحمل وسم @deprecated. واجهة tests/Consumer تستدعي getNextPosition() لإثبات استمرار دعمها.

تحقق بتجارب معدّلة مؤقتة: عند إزالة حلقة إعادة القراءة يسقط اختبار READ COMMITTED واختبار القفل المتراكم؛ وعند جعل الدالة القديمة تقفل يسقط حارس عدم القفل؛ وفي 8 تشغيلات متتالية للحالتين الخفيفة والثقيلة كان المنتظر ضحية الـ deadlock دائمًا في الأولى والمُدرِج دائمًا في الثانية، فيُمارَس الفرعان في كل تشغيل.

التحقق

مرتبط بالـ HEAD defdfabf87473e264f276ea8a218913233bd93e2.

GitHub Actions (التشغيل 38951382078، success، الفحوص العشرة ناجحة): CI Gate، وQuality (PHP 8.2)، وUnit/Regression (PHP 8.2 و8.3 و8.4 و8.5)، وIntegration (PHP 8.2 و8.5 مع MySQL 8.4.10)، وLowest dependencies (PHP 8.2 مع MySQL 8.4.10)، وWorkflow lint.

محليًا (على f7c4c28، وأضاف defdfab بعده فقرة توثيقية واحدة في PERSISTENCE_PACKAGE_REFERENCE.md فقط؛ PHP 8.4.4 وMySQL 8.4.10 حقيقي، ولا يغني عن CI): composer validate --strict وdump-autoload --optimize --strict-psr وcheck-platform-reqs وaudit --abandoned=fail وPHPStan بأعلى مستوى وunit 260 وregression 42 وintegration 119 وconsumer harness وphp-cs-fixer وgit diff --check. وأُعيدت PHPStan وphp-cs-fixer وunit وregression وintegration على حلّين جديدين للاعتماديات: الأحدث (PHPStan 2.3.1) والحد الأدنى (PHPStan 2.1.0 وphp-cs-fixer 3.94.0 وPHPUnit 11.5.0). لم يُشغَّل actionlint محليًا (Workflow lint في CI). المستودع كان يفشل في PHPStan 2.3.1 قبل هذا العمل بسبب أخطاء لا تخصه، وعولج ذلك في #35 (مدمج).

خارج النطاق

moveWithinScope() تقيّد newOrder بقيمة MAX() قديمة تحت الشرط نفسه: سُجّلت منفصلة في #33 ولم تُعدَّل هنا. #32 لم يُلمس. لا Tag ولا Release.

🤖 Generated with Claude Code

[megyptm]

Architectural review request

This PR fixes #31 but changes a documented behavior of the v1.4.0 contract, so it needs an explicit architectural decision before merge. It is not merged and no tag or release is implied.

The change in the contract

getNextPosition() was documented as "does not lock". Inside an active PDO transaction it now takes row locks (SELECT ... ORDER BY order DESC, id DESC LIMIT 1 FOR UPDATE) that are held until the caller commits or rolls back. Outside a transaction nothing changes (same plain MAX() read, no lock). Signatures, exceptions, and transaction ownership are unchanged. No lock-free fix exists: a plain read under REPEATABLE READ always uses the snapshot.

Compatibility impact

• Source compatible; behavior change only for callers that call getNextPosition() inside their own transaction.
• Those callers can now wait on locks (innodb_lock_wait_timeout, error 1205) or be chosen as a deadlock victim (1213) where before the call never blocked. The exceptions propagate unchanged.
• Callers that hold their own lock on the scope are unaffected by ordering; callers that lock in a different order than moveWithinScope() (which locks the whole scope ascending) could see new lock-order interactions.
• Callers that were already hitting the stale-snapshot duplicate get correct positions instead.

Performance impact (measured, MySQL 8.0.46 and 8.4.11, 1000-row scopes)

Access plan	Rows/records locked
Index on (scope, order), live tail	last active row, plus the gap above it
Same, 995 soft-deleted rows at the top	~997 index records and ~996 PK records (every skipped soft-deleted row)
No usable (scope, order) index (e.g. the test fixture's global table)	every row of the table
READ COMMITTED	record locks only, no gap lock

Locks are held for the caller's whole transaction, so hot scopes and long transactions amplify contention. MAX() ... FOR UPDATE was measured and rejected because it locks every row in the scope range.

What the solution does NOT guarantee

• Empty scope: no row to lock, so concurrent first inserts are not serialized. Caller must lock a parent row or use a UNIQUE constraint (test documents that both callers get 1).
• Writers that do not use locking reads are not serialized, except by the REPEATABLE READ gap lock.
• Under REPEATABLE READ the gap above the last scanned row is locked and can belong to another scope; READ COMMITTED takes no gap lock.
• No deadlock-free guarantee. Deadlock/timeout retry is out of scope (Bounded deadlock (1213) retry for runner-owned transactions #32).
• Serialization is proven only between locking participants: getNextPosition() inside a transaction and moveWithinScope() (both directions tested on real MySQL).

Decisions requested

1. Approve amending the "does not lock" contract text as written in README.md and PERSISTENCE_PACKAGE_REFERENCE.md, or reject the approach.
2. Confirm the release classification (CHANGELOG entry is under Unreleased / Fixed; versioning is the owner's call).
3. Confirm moveWithinScope() clamps newOrder to a stale MAX inside a caller transaction with an older snapshot #33 (moveWithinScope() clamps to a stale MAX()) stays a separate change.

Evidence

CI is green on HEAD 54a43a0, including CI Gate.

[megyptm]

Rework per the architectural review is pushed (HEAD d2b1907): getNextPosition() is unchanged (plain read, no lock) and the stale-snapshot fix is the additive getNextPositionForUpdate(). The earlier review-request comment describing a lock inside getNextPosition() is superseded. CI is green on this HEAD, including CI Gate (10/10 jobs). Not merged; release classification and the upstream standard wording (PACKAGE_BUILDING_STANDARD.md line 540 is still accurate, but may warrant a note about the new method) remain owner decisions.

[megyptm]

Fresh Full Acceptance Review — PR #34

Verdict: NOT READY TO MERGE

Reviewed candidate

• Base: main at 1333ae69fe1631ba5eff63168f95c5b3a17cd98f
• Head: 88b990db998fe64af2a51567be32e94f079e6e2c
• Accumulated diff: 7 commits, 7 changed files; the remote PR is open and unmerged.
• CI: run 38821058909 completed successfully, including CI Gate and the real-MySQL integration jobs.

I reviewed the cumulative code, tests, README, package reference, changelog, exception contract, PR description and previous architecture discussion against the repository instructions and pinned applicable standards. Green CI does not prove the concurrency interleaving identified below.

English flowchart — API described by this PR

flowchart TD
    A[Caller starts PDO transaction] --> B[Call getNextPositionForUpdate]
    B --> C{Scope arguments valid?}
    C -- No --> X[Throw InvalidOrderingOperationException]
    C -- Yes --> D{Transaction active on this PDO?}
    D -- No --> X
    D -- Yes --> E[Build scope and soft-delete predicates]
    E --> F[SELECT tail ORDER BY order DESC, id DESC LIMIT 1 FOR UPDATE]
    F --> G{Did the locking read wait?}
    G -- No --> H{Active tail found?}
    G -- Yes --> I[Lock on the selected row is granted]
    I --> J[Current implementation does not re-read the tail]
    J --> H
    H -- Yes --> K[Return selected order + 1]
    H -- No --> L[Return 1]
    K --> M[Caller inserts on the same PDO and transaction]
    L --> N[Caller must serialize first insert into an empty scope]
    M --> O[Caller commits or rolls back; locks are released]

Loading

The wait-and-resume branch is the gap the current tests do not prove safe.

Blocker — a waiting allocator can use a tail that is no longer the maximum

The current locking query reads and locks one tail row, then immediately returns that row's order plus one. The integration tests prove that a contending query can time out; they do not cover a waiting query that resumes successfully after another transaction inserts and commits a higher tail.

This is a correctness risk under the explicitly documented READ COMMITTED isolation level. InnoDB disables gap locking for ordinary searches/scans there, permitting inserts next to locked records. Therefore, while allocator B is waiting to lock tail row N, allocator A may be able to insert N+1 and commit. B's already-positioned descending scan may then obtain the lock on N and return N+1 without having re-evaluated that N+1 is now the tail. That can result in duplicate positions, contradicting the documented serialization guarantee for locking participants.

MySQL documentation: InnoDB Transaction Isolation Levels and Locking Reads.

Required before merge

1. Add a deterministic real-MySQL integration test for the exact interleaving: one transaction holds the old tail; a second allocator's locking read is confirmed waiting; the first inserts a higher tail and commits; the already-waiting read resumes. Assert that the waiter obtains the new maximum plus one and cannot allocate a duplicate. Cover READ COMMITTED at minimum and verify REPEATABLE READ behavior too.
2. If the test reproduces the race, fix the synchronization/algorithm or return with a narrower guarantee and an explicit owner decision. A timeout-only assertion or a new call issued after the earlier transaction ends does not cover this case.
3. Rerun the affected real-MySQL tests and applicable CI on the new remote HEAD.

Minor finding — public docs omit the SQL tie-breaker

The runtime SQL orders by both the configured order column and the configured ID column descending; the package reference mentions both, but the README and method PHPDoc summarize the query as ordering only by the order column. Align those descriptions with the actual query so maintainers understand which row is selected and locked.

Other reviewed areas

• The additive API leaves the existing getNextPosition() implementation and behavior unchanged.
• Transaction ownership, argument/scope validation, soft-delete handling, nullable scopes, exception propagation and the empty-scope limitation are represented in the implementation and surrounding documentation.
• The latest remediation restores the actual innodb_lock_wait_timeout value rather than DEFAULT, and nests cleanup so the other cleanup steps are still attempted if one step fails.
• I found no basis to treat the successful CI run as resolving the blocker above.

Final decision: NOT READY TO MERGE. Resolve the concurrency finding and the documentation mismatch, push the changes to this PR, and request a fresh full acceptance review against the updated remote base and HEAD. No merge or release is authorized by this review.

[megyptm]

Fresh Full Acceptance Review — PR #34

Verdict: NOT READY TO MERGE

Reviewed candidate

• Base: main @ 1333ae69fe1631ba5eff63168f95c5b3a17cd98f
• Head: fix/31-next-position-stale-snapshot @ 68482dce0478a65b5c1c1aa1a6aac6bd5f205700
• Accumulated diff: 10 commits, 8 changed files.
• CI: run 38824188900 completed successfully, including CI Gate, Lowest Dependencies, and both real-MySQL integration jobs.

I reviewed the full accumulated code and documentation change (not only the latest commit), the new subprocess helper, the integration/regression tests, the PR description and prior review, plus AGENTS.md, the pinned applicable standards, CONTRIBUTING.md, and the current base/HEAD comparison. The PR remains open and unmerged.

English flowchart — current API path

flowchart TD
    A[Caller starts a PDO transaction] --> B[Call getNextPositionForUpdate]
    B --> C{Scope arguments valid?}
    C -- No --> X[Throw InvalidOrderingOperationException]
    C -- Yes --> D{Transaction active on this PDO?}
    D -- No --> X
    D -- Yes --> E[Build scope and soft-delete predicates]
    E --> F[Execute SELECT tail ORDER BY order DESC, id DESC LIMIT 1 FOR UPDATE]
    F --> G{Did this read return the same tail order as the previous read?}
    G -- No --> H[Remember the current tail order]
    H --> F
    G -- Yes --> I[Return tail order + 1]
    I --> J[Caller inserts on the same PDO and transaction]
    J --> K[Caller commits or rolls back; locks are released]
    F -. Lock timeout or deadlock .-> L[Propagate PDOException; caller owns retry policy]
    D -. Scope is empty .-> M[Return 1 after two stable empty reads]
    M --> N[Caller must serialize the first insert]

Loading

The newly added READ COMMITTED regression addresses the previous stale-tail race and is a material improvement. The following findings remain.

Blocker 1 — the REPEATABLE READ regression test relies on InnoDB choosing one specific deadlock victim

In testWaitingAllocatorNeverReusesPositionUnderRepeatableRead(), the test accepts POSITION 4 or FAILURE 1213 from the waiter subprocess. However, runWaitingAllocatorBehindTailInsert() does not handle a deadlock from the parent transaction while it calls insertScoped(..., 3) or commit().

That makes the test dependent on which transaction InnoDB chooses as the deadlock victim. If the parent transaction is rolled back instead, the test can throw directly from the parent-side insert/commit, or the waiter can resume against the unchanged maximum and report POSITION 3. The test would fail even though the database avoided a duplicate. The report itself acknowledges that victim selection is not guaranteed.

MySQL 8.4 documents that InnoDB rolls back one transaction to resolve a deadlock and attempts to choose a smaller transaction, with size based on rows inserted/updated/deleted; it does not guarantee that the waiter is always the victim: Deadlock Detection and Deadlocks in InnoDB.

Required before merge: make this test deterministic with respect to either victim outcome. Handle the parent-side deadlock explicitly, then verify the committed database state and returned position for the outcome that actually occurred. Do not hide arbitrary PDOExceptions or merely broaden the accepted output; the test must still prove that no duplicate position is allocated when the higher tail commits and that a rolled-back insert was not falsely treated as committed. Repeat the test enough times to verify that it no longer depends on victim-selection luck.

Finding 2 — documented “same row twice” is not what the implementation compares

The PHPDoc and README say the locking read repeats until two consecutive reads return the same row. The implementation selects only the configured order value and compares the two integer order values ($tail === $previousTail); it does not select or compare the ID. These are not equivalent descriptions if different rows share an order value, and this library has specifically been fixing duplicate-order states.

Required: either have the query return the configured order and ID and compare the selected row identity, or change the documentation to state precisely that it repeats until two consecutive reads return the same tail order, explaining why holding the tail selected by the final locking read is sufficient for the advertised guarantee. Keep PHPDoc, README, package reference, and changelog consistent.

Finding 3 — the revised locking footprint is not fully documented

The repeated reads can lock an earlier tail and then a newer tail in the same caller-owned transaction. InnoDB retains acquired row locks until the transaction ends, so the resulting lock footprint may include a prior tail as well as the current tail. The current lock-footprint notes describe the individual scan and soft-deleted rows, but do not explain this additional lock retention caused by the new retry loop.

Required: document this consequence in the PHPDoc and public runtime behavior guidance, or otherwise demonstrate that the implementation releases obsolete tail locks without breaking the concurrency guarantee. Do not describe the behavior as locking only the final tail if multiple selected tail rows remain locked.

Minor repository-facing accuracy issue

The PR description still says ScopedOrderingManagerStaleSnapshotTest has 19 tests and the integration suite has 113 tests. The current handoff reports 21 tests in that class and 115 integration tests. Refresh the PR description so its test counts and verification claims match the reviewed HEAD. The local report also says composer validate, composer audit, and actionlint were not run locally; state that accurately if the description lists local gates, while retaining the separate evidence for green GitHub CI.

Other areas reviewed

• The existing getNextPosition() remains a plain read, preserving its existing public behavior.
• The additive locking API validates its transaction precondition and shares scope/soft-delete predicate construction.
• The README, package reference, PHPDoc, and changelog now describe the new repeated-read behavior and the possible deadlock under REPEATABLE READ.
• The session-timeout cleanup remediation from the previous review remains present.
• All ten jobs in the latest reported CI run passed. CI success does not make a known non-deterministic concurrency test acceptable for a public library.

Final decision: NOT READY TO MERGE. Resolve Blocker 1, align the same-row/same-order contract, document the repeat-read lock footprint, refresh the stale PR description, push the result to this PR, and request another Fresh Full Acceptance Review against the new remote HEAD. No merge or release is authorized by this review.

[megyptm]

Fresh Full Acceptance Review — PR #34

الحكم: جاهزة للدمج هندسيًا — لا يوجد blocker (مع إصلاح توثيقي صغير موصى به قبل الدمج)

المرشح الذي رُوجع

• Base: main @ 1333ae69fe1631ba5eff63168f95c5b3a17cd98f (الفرع غير متأخر عن main: behind 0 / ahead 16).
• Head: fix/31-next-position-stale-snapshot @ f7c4c28a2f53cdcbe7a35069df77f86c2d64bbb7.
• الـ diff المتراكم: 16 commit، 8 ملفات (+1042 / −19). الـ PR مفتوحة وغير Draft، وmergeStateStatus = CLEAN.
• CI: التشغيل 38911474116 ناجح بالكامل (10/10 بما فيها CI Gate وIntegration على MySQL 8.4.10 وLowest dependencies).

راجعت الحالة المتراكمة كاملة، لا آخر commit فقط: الكود، والاختبارات، وسكربت الـ subprocess، وREADME، وPERSISTENCE_PACKAGE_REFERENCE.md، وCHANGELOG، وعقد الاستثناء، ووصف الـ PR، والمراجعتين السابقتين، مقابل AGENTS.md والمعايير المنطبقة المسجلة في STANDARDS_MANIFEST.md (Adoption، وPACKAGE_BUILDING، وTESTING، وLIBRARY_PRESENTATION، وCOMPOSER_PACKAGE، وAI_COLLABORATION_WORKFLOW، وPhase Stack) وCONTRIBUTING.md.

Flowchart — مسار الـ API الحالي

flowchart TD
    A[Caller starts a PDO transaction] --> B[Call getNextPositionForUpdate]
    B --> C{Scope arguments valid?}
    C -- No --> X[Throw InvalidOrderingOperationException]
    C -- Yes --> D{Transaction active on this PDO?}
    D -- No --> X
    D -- Yes --> E[Build scope and soft-delete WHERE]
    E --> F[SELECT order ... ORDER BY order DESC, id DESC LIMIT 1 FOR UPDATE]
    F --> G{Same order value as previous read?}
    G -- No --> H[Remember tail order]
    H --> F
    G -- Yes --> I[Return tail + 1, or 1 for empty scope]
    I --> J[Caller inserts on the same PDO and transaction]
    J --> K[Caller commits or rolls back; all tail locks released]
    F -. Lock wait timeout or deadlock .-> L[PDOException propagates; caller owns retry]

Loading

التحقق من الـ remediations السابقة

الملاحظة السابقة	الحالة
منتظر يستأنف بعد تثبيت ذيل أعلى تحت READ COMMITTED فيعيد موضعًا مكررًا	مُصلحة. حلقة إعادة القراءة القافلة حتى تتطابق قراءتان. تحققت بـ mutation محلي: عند استبدال الحلقة بقراءة واحدة يسقط testWaitingAllocatorSeesHigherTailCommittedWhileItWaitedUnderReadCommitted وtestRepeatedLockingReadKeepsEveryTailItLockedUntilTheTransactionEnds، وعند إعادتها ينجحان.
اختبار REPEATABLE READ يعتمد على ضحية deadlock بعينها	مُصلحة. يُلتقط 1213 من جهة المعاملة المُدرِجة فقط (أي خطأ آخر يُعاد رميه)، ثم تُقرأ الحالة المثبّتة من اتصال مستقل وتُطابق مع النتيجة الفعلية، مع حالتين (light / heavy). في 5 تشغيلات محلية متتالية تراوح عدد الـ assertions بين 125 و126، فالفرعان مُمارَسان فعلًا.
وصف «same row» لا يطابق المقارنة الفعلية	مُصلحة. كل الوثائق تقول الآن «same order value».
أقفال الذيول المتراكمة غير موثقة	مُصلحة. موثقة في PHPDoc وREADME والمرجع، ومحمية باختبار يفحص أن الذيلين مقفولان وأن الصف الأقدم غير مقفول.
tie-breaker الـ id غائب عن الوثائق	مُصلحة.
أعداد الاختبارات في وصف الـ PR	مطابقة. 25 اختبارًا في الكلاس، وunit 260 وregression 42 وintegration 119 — طابقت تشغيلي المحلي.

مراجعة التصميم والعقد

• عقد v1.4.0 محفوظ: getNextPosition() لم يتغير SQL أو سلوكه (استُخرج فقط buildScopeWhere())، والإضافة الوحيدة على الواجهة العامة هي دالة جديدة، ويثبتها OrderingPublicApiRegressionTest. لا breaking change، ولا تغيير في ملكية المعاملات أو savepoint.
• صحة الخوارزمية: بعد كل قراءة قافلة تمسك المعاملة قفل الذيل الذي قرأته، فلا يستطيع أي مشارك يستخدم قراءات قافلة تحريك الذيل. لذلك تطابق قراءتين متتاليتين يعني أن الموضع المُعاد مشتق من ذيل مقفول. الحدود الوحيدة (النطاق الفارغ، والكاتبون بلا قراءات قافلة، وتكرار قيمة الترتيب نفسها في صفوف قائمة) موثقة صراحة.
• التكامل مع savepoint: ROLLBACK TO SAVEPOINT في InnoDB لا يحرر الأقفال، وهذا يتسق مع عبارة «the lock is held until the caller ends the transaction».
• استخدام InvalidOrderingOperationException لشرط مسبق غير مستوفى (المعاملة غير النشطة): اختيار مقبول وموثق في الكلاس والمرجع، ولا يضيف استثناءً جديدًا للواجهة.
• نمط قراءة fetchColumn() يطابق النمط المحلي القائم في findCurrentOrder()، ويمر في PHPStan بأعلى مستوى.

الملاحظات

1. (Minor — توثيق، موصى بإصلاحه في هذه الـ PR) عبارة متبقية تكرر الافتراض الذي أسقطه #31

في PERSISTENCE_PACKAGE_REFERENCE.md، قسم Transaction Composition، ما زال البند الخاص بـ getNextPosition() يقول:

The caller owns the transaction and locking needed to serialize concurrent position allocation.

تحت REPEATABLE READ لا يكفي قفل المستدعي إذا أُخذ بعد أول قراءة غير قافلة، وهذا تحديدًا سبب #31. وقد وثّقت الـ PR هذه القاعدة بدقة في البند الخاص بالدالة نفسها وفي README، لكن هذا الملخص لم يُحدَّث، فيعطي القارئ الذي يكتفي به التوجيه الخاطئ نفسه. المقترح: إضافة قيد مثل «… taken before the transaction's first non-locking read; otherwise use getNextPositionForUpdate()» (LIBRARY_PRESENTATION §12، وAI_COLLABORATION §13.3 «كل claim مطابق للحالة الحالية»).

2. (Nit — خارج النطاق عمدًا) getMaxPosition() لا تستخدم buildScopeWhere()

ما زالت getMaxPosition() تبني الـ WHERE يدويًا رغم وجود الـ helper الجديد. لا أثر سلوكي. أوصي بتركه لـ #33 الذي سيعدّل getMaxPosition() أصلًا، حفاظًا على نطاق هذه الـ PR.

3. (ملاحظة — لا يلزم تغيير) لا يوجد حد أقصى لعدد التكرارات

تتوقف الحلقة بمجرد أن يكف الذيل عن التغير. مع المشاركين الذين يستخدمون قراءات قافلة تنتهي بعد قراءتين أو ثلاث. وحده تيار مستمر من إدراجات كتّاب لا يستخدمون قراءات قافلة تحت READ COMMITTED يمكن أن يطيلها، وكل تكرار فيه يمثل تقدمًا حقيقيًا. السلوك موثق («continues only while the tail keeps changing»).

4. (ملاحظة بيئية) اختبار REPEATABLE READ يفترض innodb_deadlock_detect = ON

هذه القيمة الافتراضية في MySQL وفي CI. إذا عُطّلت في بيئة ما فسيصل الإدراج إلى 1205 ويفشل الاختبار بخطأ لا بنتيجة خاطئة. لا يلزم تغيير.

5. (تحسين اختياري) Consumer Harness

غيّرت الـ PR مسار التخصيص الأساسي في Quick Start إلى getNextPositionForUpdate() داخل PdoTransactionRunner، بينما ما زال tests/Consumer/run.php يمارس getNextPosition() فقط. هذا لا يخالف TESTING_STANDARD، فاختبارات Integration تمر عبر الواجهة العامة على MySQL حقيقي، والـ Harness يغطي workflow واقعيًا. لكن إضافة استدعاء الدالة الجديدة إلى الـ Harness تجعل الإثبات الخارجي مطابقًا للمسار الموثق الآن.

التحقق المحلي المستقل

شغّلت الأوامر على نسخة نظيفة من HEAD f7c4c28 باستخدام PHP 8.4.4 وMySQL 8.4.10 حقيقي في container مؤقت (REPEATABLE-READ، innodb_deadlock_detect = 1). لا يغني هذا عن CI:

• composer validate --strict، وdump-autoload --optimize --strict-psr، وcheck-platform-reqs، وaudit --abandoned=fail: نجحت.
• composer analyse (PHPStan max): No errors.
• test:unit: ‏260 اختبارًا OK. وtest:regression: ‏42 اختبارًا OK. وtest:integration: ‏119 اختبارًا OK.
• ScopedOrderingManagerStaleSnapshotTest في 5 تشغيلات متتالية: كلها OK.
• test:consumer: نجح التشغيلان النظيفان.
• php-cs-fixer check: 0 ملفات تحتاج إصلاحًا.
• Mutation (إزالة حلقة إعادة القراءة): سقط اختباران كما هو متوقع، ثم أُعيد الكود.
• لم أشغّل actionlint محليًا (يغطيه Workflow lint في CI).

قرارات المالك المتبقية (ليست blockers هندسية)

1. تصنيف الإصدار: الإضافة متوافقة وبمستوى minor. لا Tag ولا Release ضمن هذه الـ PR.
2. توجيه PACKAGE_BUILDING_STANDARD.md (السطر 540 في النسخة المثبتة): «the caller owns both the transaction and the appropriate locking strategy» ناقص في ضوء قاعدة الـ snapshot. هذا تعديل upstream في Maatify/php-engineering-standards، ولا يجوز تعديل النسخة المثبتة محليًا.
3. يبقى moveWithinScope() clamps newOrder to a stale MAX inside a caller transaction with an older snapshot #33 (moveWithinScope() وقيمة MAX() القديمة) تغييرًا منفصلًا، ولم يُلمس Bounded deadlock (1213) retry for runner-owned transactions #32.

القرار النهائي: لا blocker، والحالة المتراكمة جاهزة للدمج هندسيًا. أوصي بتطبيق الملاحظة 1 (تعديل نصي من سطر واحد) قبل الدمج. قرار الدمج للمالك، ولا تمنح هذه المراجعة تصريحًا بأي Merge أو Tag أو Release.

🤖 Generated with Claude Code