You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "Colafornia (via GitHub)" <gi...@apache.org> on 2026/09/01 07:36:15 UTC

[PR] fix(runtime-host): preserve durable event order [maka]

Colafornia opened a new pull request, #4445:
URL: https://github.com/apache/maka/pull/4445

   ## Summary
   
   Branch and revision copies used timestamp-sorted RuntimeEvents when validating history-compaction checkpoints. If timestamps differed from durable append order, valid checkpoints could be rejected or dropped.
   
   Read RuntimeEvents in durable ordinal order for branch and revision copies, while keeping the existing inline-run boundary and side-conversation behavior.
   
   Fixes #4103
   
   ## Verification
   
   - `npm run lint`
   - `npm run format:check`
   - `npm --workspace @maka/runtime-host run typecheck`
   - `npm --workspace @maka/runtime-host run test:dist`: 1495 passed, 0 failed, 12 skipped
   - Knip checks passed for `apps/desktop` and `packages/ui`.
   - ASF headers and renderer architecture checks passed.
   - Full workspace typecheck remains blocked by unrelated existing UI/desktop type errors.
   - Native peer fmt/clippy checks were not run because `cargo` is unavailable in the environment.
   
   ## AI use
   
   - [ ] No generative tool made a substantive contribution
   - [x] Generative tooling made a substantive contribution
   
   Tool(s) and scope: OpenAI Codex — diagnosis, implementation, and tests.
   
   ## Checklist
   
   - [x] Tests cover the change and fail without it
   - [x] Lint, format, typecheck and the affected suites pass locally
   
   Does this PR entail a change in behavior?
   
   - [x] Yes — branch and revision copies now preserve durable RuntimeEvent order
   - [ ] No


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] fix(runtime-host): preserve durable event order [maka]

Posted by "Colafornia (via GitHub)" <gi...@apache.org>.
Colafornia commented on PR #4445:
URL: https://github.com/apache/maka/pull/4445#issuecomment-5505790832

   @Astro-Han  Thanks for the review. Changed the approach to enforce durable event ordering in `RuntimeReadModel`.
   However, we cannot use runIndex globally because it is not a durable session ordinal and breaks graph revision ordering.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] fix(runtime): preserve durable RuntimeEvent order [maka]

Posted by "Colafornia (via GitHub)" <gi...@apache.org>.
Colafornia commented on PR #4445:
URL: https://github.com/apache/maka/pull/4445#issuecomment-5508046491

   Addressed in 4f85c3def.
   
   - Fixed at the owner: immutable session ordinals are required and drive the read-model order. The memory stores now exercise the production path.
   - Simplified: removed the timestamp-slot permutation and replaced it with one comparator. Mutable partials keep their adjacent per-run position; unanchored legacy rows use timestamp fallback after durable rows.
   - Fixed: terminal partial snapshots are read from the complete per-run ledger.
   - Fixed: copied inline events are imported in source session order, including cross-run interleaving.
   - Updated #4103 and the PR body: since #4348, the reachable defect is checkpoint loss and target-order drift, not persistence_failed.
   - Deferred: filtering ordinal reads to inline run IDs needs a store API change and has no measured impact yet.
   - Kept unchanged: readSessionRuntimeEvents includes mutable presentation state and serves repair/trace consumers; readSessionRuntimeEventEntries is the immutable ordering API.
   
   Verification: affected runtime files 232/232, SQLite runtime store 47/47, two-client UDS 1/1, four package builds, Biome, and git diff check.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] fix(runtime-host): preserve durable event order [maka]

Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4445:
URL: https://github.com/apache/maka/pull/4445#discussion_r3912324748


##########
packages/runtime/src/runtime-read-model.ts:
##########
@@ -250,6 +268,24 @@ export class RuntimeReadModel {
         a.event.id.localeCompare(b.event.id),
     );
 
+    const nextFallbackEventByRun = new Map<number, number>();
+    for (let index = 0; index < ordered.length; index += 1) {
+      const item = ordered[index]!;
+      if (item.ordinal !== undefined) continue;
+      const next = nextFallbackEventByRun.get(item.runIndex) ?? 0;
+      ordered[index] = fallbackEventsByRun.get(item.runIndex)![next]!;
+      nextFallbackEventByRun.set(item.runIndex, next + 1);
+    }
+
+    const durableEvents = ordered
+      .filter((item) => item.ordinal !== undefined)
+      .sort((a, b) => a.ordinal! - b.ordinal! || a.event.id.localeCompare(b.event.id));
+    let nextDurableEvent = 0;
+    for (let index = 0; index < ordered.length; index += 1) {
+      if (ordered[index]!.ordinal === undefined) continue;
+      ordered[index] = durableEvents[nextDurableEvent++]!;
+    }
+
     return this.buildView({
       runs: inlineRuns,
       events: ordered.map((item) => item.event),

Review Comment:
   This is a third order: a durable event's final position is decided by some other event's `ts`, and a fallback event can land between two consecutively appended durable events. If ordinal is the owner, one comparator (`ordinal`, then `ts` for rows without one) replaces both passes.



##########
packages/core/src/runtime-event-store.ts:
##########
@@ -96,6 +96,10 @@ export interface RuntimeEventStore {
     event: RuntimeEvent,
   ): Promise<void>;
   readRuntimeEvents(sessionId: string, runId: string): Promise<RuntimeEvent[]>;
+  /** Session-wide immutable append order when the store exposes it. */
+  readSessionRuntimeEventEntries?(

Review Comment:
   Both production stores implement this; only test doubles do not. Required here, and the no-ordinal branch in the read model goes away.



##########
packages/runtime/src/runtime-read-model.ts:
##########
@@ -250,6 +268,24 @@ export class RuntimeReadModel {
         a.event.id.localeCompare(b.event.id),
     );
 
+    const nextFallbackEventByRun = new Map<number, number>();
+    for (let index = 0; index < ordered.length; index += 1) {
+      const item = ordered[index]!;
+      if (item.ordinal !== undefined) continue;
+      const next = nextFallbackEventByRun.get(item.runIndex) ?? 0;
+      ordered[index] = fallbackEventsByRun.get(item.runIndex)![next]!;
+      nextFallbackEventByRun.set(item.runIndex, next + 1);
+    }
+
+    const durableEvents = ordered

Review Comment:
   With every event carrying an ordinal (the sqlite normal case) every slot is durable and the refill below turns the whole view into ordinal order; `ts` stops mattering. That changes the overlapping-run interleave pinned at `session-manager.test.ts:8879` for every production session, and that test's memory store has no ordinal reader, so it cannot see it. Say it is intended and give the memory store the reader, or keep `ts`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] fix(runtime): preserve durable RuntimeEvent order [maka]

Posted by "Colafornia (via GitHub)" <gi...@apache.org>.
Colafornia commented on code in PR #4445:
URL: https://github.com/apache/maka/pull/4445#discussion_r3913052599


##########
packages/runtime/src/runtime-read-model.ts:
##########
@@ -250,6 +268,24 @@ export class RuntimeReadModel {
         a.event.id.localeCompare(b.event.id),
     );
 
+    const nextFallbackEventByRun = new Map<number, number>();
+    for (let index = 0; index < ordered.length; index += 1) {
+      const item = ordered[index]!;
+      if (item.ordinal !== undefined) continue;
+      const next = nextFallbackEventByRun.get(item.runIndex) ?? 0;
+      ordered[index] = fallbackEventsByRun.get(item.runIndex)![next]!;
+      nextFallbackEventByRun.set(item.runIndex, next + 1);
+    }
+
+    const durableEvents = ordered

Review Comment:
   Accepted in 4f85c3def. Production presentation now intentionally follows immutable session append order. MemoryAgentRunStore records the same global ordinals, and the overlapping-run test now exercises and names durable session chronology. The PR and issue descriptions state the behavior change.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] fix(runtime): preserve durable RuntimeEvent order [maka]

Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4445:
URL: https://github.com/apache/maka/pull/4445#discussion_r3915441383


##########
packages/runtime/src/runtime-read-model.ts:
##########
@@ -504,3 +512,58 @@ function isTerminalRunStatus(status: AgentRunHeader['status']): boolean {
 function errorMessage(error: unknown): string {
   return error instanceof Error ? error.message : String(error);
 }
+
+interface OrderedRuntimeEvent {
+  event: RuntimeEvent;
+  runIndex: number;
+  eventIndex: number;
+  ordinal?: number;
+  ordinalPhase?: -1 | 0 | 1;
+}
+
+function appendOrderedEvents(
+  ordered: OrderedRuntimeEvent[],
+  events: readonly RuntimeEvent[],
+  runIndex: number,
+  ordinals?: ReadonlyMap<string, number>,
+): void {
+  const nextOrdinals: Array<number | undefined> = new Array(events.length);
+  let nextOrdinal: number | undefined;
+  for (let eventIndex = events.length - 1; eventIndex >= 0; eventIndex -= 1) {
+    nextOrdinal = ordinals?.get(events[eventIndex]!.id) ?? nextOrdinal;
+    nextOrdinals[eventIndex] = nextOrdinal;
+  }
+  let previousOrdinal: number | undefined;
+  for (let eventIndex = 0; eventIndex < events.length; eventIndex += 1) {
+    const event = events[eventIndex]!;
+    const durableOrdinal = ordinals?.get(event.id);
+    if (durableOrdinal !== undefined) previousOrdinal = durableOrdinal;
+    const ordinal = durableOrdinal ?? previousOrdinal ?? nextOrdinals[eventIndex];
+    const ordinalPhase = durableOrdinal !== undefined ? 0 : previousOrdinal !== undefined ? 1 : -1;
+    ordered.push({
+      event,
+      runIndex,
+      eventIndex,
+      ...(ordinal !== undefined ? { ordinal, ordinalPhase } : {}),
+    });
+  }
+}
+
+function compareOrderedRuntimeEvents(a: OrderedRuntimeEvent, b: OrderedRuntimeEvent): number {
+  if (a.ordinal !== undefined || b.ordinal !== undefined) {

Review Comment:
   Ordinals are `MAX(ordinal) + 1` at append, so a single-run repair backfill lands here with the highest ordinals and that run's tail sinks to the end of the session, on disk. `main`'s `ts` order handled this. Anchor by the run's smallest known ordinal instead, or state in the body why sinking is acceptable.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] fix(runtime): preserve durable RuntimeEvent order [maka]

Posted by "Colafornia (via GitHub)" <gi...@apache.org>.
Colafornia commented on code in PR #4445:
URL: https://github.com/apache/maka/pull/4445#discussion_r3921279745


##########
packages/runtime/src/runtime-read-model.ts:
##########
@@ -504,3 +512,58 @@ function isTerminalRunStatus(status: AgentRunHeader['status']): boolean {
 function errorMessage(error: unknown): string {
   return error instanceof Error ? error.message : String(error);
 }
+
+interface OrderedRuntimeEvent {
+  event: RuntimeEvent;
+  runIndex: number;
+  eventIndex: number;
+  ordinal?: number;
+  ordinalPhase?: -1 | 0 | 1;
+}
+
+function appendOrderedEvents(
+  ordered: OrderedRuntimeEvent[],
+  events: readonly RuntimeEvent[],
+  runIndex: number,
+  ordinals?: ReadonlyMap<string, number>,
+): void {
+  const nextOrdinals: Array<number | undefined> = new Array(events.length);
+  let nextOrdinal: number | undefined;
+  for (let eventIndex = events.length - 1; eventIndex >= 0; eventIndex -= 1) {
+    nextOrdinal = ordinals?.get(events[eventIndex]!.id) ?? nextOrdinal;
+    nextOrdinals[eventIndex] = nextOrdinal;
+  }
+  let previousOrdinal: number | undefined;
+  for (let eventIndex = 0; eventIndex < events.length; eventIndex += 1) {
+    const event = events[eventIndex]!;
+    const durableOrdinal = ordinals?.get(event.id);
+    if (durableOrdinal !== undefined) previousOrdinal = durableOrdinal;
+    const ordinal = durableOrdinal ?? previousOrdinal ?? nextOrdinals[eventIndex];
+    const ordinalPhase = durableOrdinal !== undefined ? 0 : previousOrdinal !== undefined ? 1 : -1;
+    ordered.push({
+      event,
+      runIndex,
+      eventIndex,
+      ...(ordinal !== undefined ? { ordinal, ordinalPhase } : {}),
+    });
+  }
+}
+
+function compareOrderedRuntimeEvents(a: OrderedRuntimeEvent, b: OrderedRuntimeEvent): number {
+  if (a.ordinal !== undefined || b.ordinal !== undefined) {

Review Comment:
   Keeping tail placement as the append-order contract. A repair write occurs after the existing session events, so its new ordinal is intentionally later. Anchoring the whole run at its smallest ordinal would change known `A1 → B1 → A2` interleaving into `A1 → A2 → B1`, which contradicts the ordinal authority established by this PR. The PR body now states that a repaired run can appear split because timestamps cannot recover a trustworthy cross-run insertion point.
   
   The two reductions are in `c60400d80`: `ordinalPhase` is removed, and `importConversationCopyRuntimeEvents` is required with the order-breaking fallback deleted. The focused 70 tests and `@maka/runtime` typecheck pass on the rebased head.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] fix(runtime): preserve durable RuntimeEvent order [maka]

Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4445:
URL: https://github.com/apache/maka/pull/4445#discussion_r3921310803


##########
packages/runtime/src/runtime-read-model.ts:
##########
@@ -504,3 +512,53 @@ function isTerminalRunStatus(status: AgentRunHeader['status']): boolean {
 function errorMessage(error: unknown): string {
   return error instanceof Error ? error.message : String(error);
 }
+
+interface OrderedRuntimeEvent {
+  event: RuntimeEvent;
+  runIndex: number;
+  eventIndex: number;
+  ordinal?: number;
+}
+
+function appendOrderedEvents(
+  ordered: OrderedRuntimeEvent[],
+  events: readonly RuntimeEvent[],
+  runIndex: number,
+  ordinals?: ReadonlyMap<string, number>,
+): void {
+  const nextOrdinals: Array<number | undefined> = new Array(events.length);

Review Comment:
   P3, not blocking. I deleted this backward pass and the `?? nextOrdinals[eventIndex]` on line 540, rebuilt, and the runtime suite is still 3167/3167. It only fires for a mutable partial that precedes every durable event in its run, and a run's opening event is immutable, so I could not construct that case either. Either cover it or drop the seven lines; leaving it uncovered is the thing I would avoid.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] fix(runtime): preserve durable RuntimeEvent order [maka]

Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #4445:
URL: https://github.com/apache/maka/pull/4445


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org