From 89487db72ace63d1ef8e7a7ddbe60c0dc14a5e02 Mon Sep 17 00:00:00 2001 From: kami Date: Tue, 9 Jun 2026 10:14:04 +0400 Subject: [PATCH] fix(kernel,router): rehydrate by CAS content hash (F-021) + narration budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rehydrate() now scans ArtifactContentStoredEvent (the durable slot->CAS-hash bridge) and fetches stored bytes by content hash, keying the cache by the logical slot name. The slot name is never passed to the hash-keyed CAS, so the odd-length-hex crash is gone and artifact content is restored on cold-start resume. RehydrateTest reworked to use a content-addressed fake store (slot name != content hash) so it actually exercises the bug. Replay (#8) is unaffected — rehydrate is live-resume-only. Also bump router narration maxTokens 1024 -> 4096: reasoning models were spending the whole budget thinking and emitting empty content (finishReason= length), so narrations never reached the TUI. Removes the stale TUI-refactor progress doc. --- .../DefaultSessionOrchestrator.kt | 13 +- .../correx/core/router/model/RouterConfig.kt | 9 +- .../plans/2026-05-17-tui-refactor.progress.md | 130 ------------------ qa/audit-report-2026-06-08.md | 14 +- .../src/test/kotlin/RehydrateTest.kt | 32 +++-- 5 files changed, 42 insertions(+), 156 deletions(-) delete mode 100644 docs/plans/2026-05-17-tui-refactor.progress.md diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestrator.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestrator.kt index d305ecbf..c2e4ef95 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestrator.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestrator.kt @@ -10,6 +10,7 @@ import com.correx.core.artifacts.ArtifactState import com.correx.core.artifactstore.ArtifactStore import com.correx.core.events.events.ApprovalDecisionResolvedEvent import com.correx.core.events.events.ApprovalRequestedEvent +import com.correx.core.events.events.ArtifactContentStoredEvent import com.correx.core.events.events.ArtifactValidatedEvent import com.correx.core.events.events.OrchestrationResumedEvent import com.correx.core.events.events.RefinementIterationEvent @@ -164,15 +165,17 @@ class DefaultSessionOrchestrator( /** * Rebuilds [artifactContentCache] from durable events after a server restart. - * Scans all events for [ArtifactValidatedEvent] and re-fetches the stored bytes - * from [artifactStore] for each one. Must be called before [resume] to ensure - * transition conditions that read artifact content work correctly. + * Scans all events for [ArtifactContentStoredEvent], which records the + * slot-name -> CAS content-hash mapping at write time, and re-fetches the + * stored bytes from [artifactStore] by **content hash** (not slot name). + * Must be called before [resume] so transition conditions and `needs`-injection + * that read artifact content work correctly after a cold start. */ suspend fun rehydrate(sessionId: SessionId) { repositories.eventStore.readFrom(sessionId, fromSequence = 0L).forEach { event -> - val p = event.payload as? ArtifactValidatedEvent ?: return@forEach + val p = event.payload as? ArtifactContentStoredEvent ?: return@forEach val key = "${sessionId.value}:${p.artifactId.value}" - artifactStore.get(p.artifactId) + artifactStore.get(p.contentHash) ?.toString(Charsets.UTF_8) ?.let { artifactContentCache[key] = it } } diff --git a/core/router/src/main/kotlin/com/correx/core/router/model/RouterConfig.kt b/core/router/src/main/kotlin/com/correx/core/router/model/RouterConfig.kt index 817f49b2..7c697061 100644 --- a/core/router/src/main/kotlin/com/correx/core/router/model/RouterConfig.kt +++ b/core/router/src/main/kotlin/com/correx/core/router/model/RouterConfig.kt @@ -15,12 +15,13 @@ data class RouterConfig( maxTokens = 512, ), // Narration uses a larger token ceiling: reasoning models spend tokens "thinking" - // before emitting content, and a 512 cap left no room for the actual line (the - // response came back empty with finishReason=length). 1024 lets thinking complete - // and still produce the one or two sentences. + // before emitting content, and a tighter cap left no room for the actual line (the + // response came back empty with finishReason=length). 1024 still wasn't enough — + // the model burned the whole budget reasoning and emitted no content — so 4096 + // gives reasoning room to complete and still produce the one or two sentences. val narrationGenerationConfig: GenerationConfig = GenerationConfig( temperature = 0.7, topP = 0.9, - maxTokens = 1024, + maxTokens = 4096, ), ) diff --git a/docs/plans/2026-05-17-tui-refactor.progress.md b/docs/plans/2026-05-17-tui-refactor.progress.md deleted file mode 100644 index 38f428b8..00000000 --- a/docs/plans/2026-05-17-tui-refactor.progress.md +++ /dev/null @@ -1,130 +0,0 @@ -# TUI Refactor — Progress Log - -**Plan:** `docs/plans/2026-05-17-tui-refactor.md` -**Last update:** 2026-05-17 -**Mode:** running with `/implement` (spec + quality review skipped per user instruction) -**Base SHA at start:** `58670a6` (no commits made yet — all changes still in working tree) - ---- - -## Completed - -### Task 1.1 — Upgrade Mosaic to 0.18.0 ✅ -- `apps/tui/build.gradle`: mosaic-runtime 0.13.0 → 0.18.0 -- **Scope creep:** Mosaic 0.18 requires Kotlin 2.2.10, so root `build.gradle` was bumped 2.0.21 → 2.2.10 globally. -- Side-effect: 9 new `MaxLineLength` detekt failures in `core/kernel/orchestration/{SessionOrchestrator,DefaultSessionOrchestrator}.kt` — fixed by wrapping long `System.err.println` calls. Added `LongMethod` to existing `@SuppressWarnings` on `SessionOrchestrator`. -- `./gradlew check` green. - -### Task 1.2 — Action sealed type + KeyResolver ✅ -- Created `apps/tui/.../input/Action.kt` (14 variants), `input/KeyResolver.kt`, `test/.../input/KeyResolverTest.kt` (18 tests). -- Added `Filter` to `KeyEvent.kt`. -- Added JUnit 5 + kotlin-test deps + `useJUnitPlatform()` to `apps/tui/build.gradle`. - -### Task 1.3 — Replace stdin reader with `Modifier.onKeyEvent` ✅ -- Deleted `keyChannel`, `readKeys`, `charToKeyEvent`, `handleKeys`, `handleInputModeKey`. -- Created `input/MosaicKeyMapper.kt` mapping `com.jakewharton.mosaic.layout.KeyEvent { key, alt, ctrl, shift }` → domain `KeyEvent`. -- `Column` now has `Modifier.onKeyEvent { ... }` at root; dispatch path = MosaicKeyMapper → KeyResolver → `applyAction`. -- `applyAction(action, state, wsClient, scope): TuiState` is pure on state; ws sends launched via `scope.launch` from within (transitional — Task 2.3 replaces it). -- `TuiNavigation.kt` shrunk: kept `navigateUp`/`navigateDown`, dropped `parseEscapeSequence` + `handleCancel`. - -### Task 1.4 — Steering note inside InputMode ✅ -- `InputMode` final shape: `None, WorkflowId, SteeringNote, Filter`. -- `Action.OpenSteeringPrompt` → switches to `SteeringNote` only if `state.approval.active != null`. -- `Action.SubmitInput` with mode=SteeringNote → sends `ApprovalResponse(decision=STEER, steeringNote=text)`, clears approval. -- `InputBar` labels updated. `handleSteer` deleted. No `System.console` anywhere in `apps/tui`. - -### Task 2.1 — Split TuiState into slices ✅ -- New: `state/{ConnectionState,SessionsState,InputState,ApprovalState,ProviderState}.kt`. -- `TuiState` is now composition of the five slices with defaults. -- Field mapping applied across `TuiApp.kt`, `TuiNavigation.kt`, `StateReducer.kt`, `components/StatusBar.kt`: - - `state.connected/reconnecting` → `state.connection.*` - - `state.sessions` (list) → `state.sessions.sessions`; `state.selectedSessionId` → `state.sessions.selectedId` - - `state.activeApproval` → `state.approval.active` - - `state.inputText/inputMode` → `state.input.text/mode` - - `state.providerId/providerStatus` → `state.provider.id/status` - -### Task 2.2 — Pure RootReducer + Effect ✅ -- New: `reducer/{Effect,InputReducer,SessionsReducer,ApprovalReducer,ConnectionReducer,ProviderReducer,ServerMessageReducer,RootReducer}.kt`. -- `Action` extended: `ServerEventReceived(ServerMessage)`, `Connected`, `Disconnected`, `RetryScheduled(attempt, nextRetryAtMs)`. -- All slice reducers return `Pair>`. Default arm = `state to emptyList()`. -- 39 new tests added (55 total in module). -- **NOT YET WIRED** into `TuiApp.kt` — that is Task 2.3. The old `applyAction`/`applyServerMessage` paths still run. -- Decisions deviating from plan: dropped unused `selectedSessionId` param on `InputReducer.reduce`; dropped unused `clock` param on `ConnectionReducer.reduce`. Old `StateReducer.kt` left in place alongside new `ServerMessageReducer.kt` (no callers of `ServerMessageReducer` yet). - -### Task 2.3 — EffectDispatcher + wire RootReducer into TuiApp ✅ -- Create `reducer/EffectDispatcher.kt`. -- Rewrite `ws/TuiWsClient.kt`: expose `messages: SharedFlow` and `connection: SharedFlow`; remove suspend-callback constructor params. Add `sealed interface ConnectionEvent { Connected, Disconnected, RetryScheduled(attempt, nextRetryAtMs) }`. -- In `TuiApp`: single `dispatch(action)` → `RootReducer.reduce` → `Snapshot.withMutableSnapshot { state = next }` → trySend each `Effect` to a `Channel`. -- Launch coroutines: `ws.connect()`, collect `ws.messages` → `dispatch(ServerEventReceived(it))`, collect `ws.connection` → `dispatch(it.toAction())`, drain effects channel via `EffectDispatcher`. -- Delete old `applyAction`/`applyServerMessage` (and `StateReducer.kt` if unused after the cut). -- Smoke-test: `n` → workflow id prompt → submit → session starts; approval flow; steering note. - -### Task 3.1 — `Panel` composable + `TerminalSize` ✅ -- Create `components/Panel.kt` (`@Composable fun Panel(title: String?, content: @Composable () -> Unit)`). -- Create `components/TerminalSize.kt` (data class + `staticCompositionLocalOf { TerminalSize(80, 24) }`). -- Do not wire yet. - -### Task 3.2 — Migrate components to `Panel` ✅ -- Wrap each section in `TuiApp` with `Panel(title=…) { … }`. -- Drop every `BOX_WIDTH` constant and hand-padding from `StatusBar`, `SessionList`, `ActiveSession`, `ApprovalPanel`, `InputBar`. -- `grep -r "BOX_WIDTH" apps/tui` must return nothing. - -### Task 3.3 — Responsive terminal width ✅ -- Add `readTerminalSize()` (stty fallback, default 80×24) in `TerminalSize.kt`. -- 500ms polling loop in `TuiApp.runTuiApp`; wrap content in `CompositionLocalProvider(LocalTerminalSize provides …)`. - -### Task 4.2 — Session filter via `InputMode.Filter` ✅ -- `/` enters Filter mode (already wired in resolver). On `SubmitInput` mode=Filter, copy text into `sessions.filter`. On `CancelInput` mode=Filter, clear filter. -- `SessionList` renders only sessions whose `workflowId` contains `filter` (case-insensitive). - -### Task 4.3 — Reconnect feedback ✅ -- In `TuiWsClient.connect`, before backoff `delay`, emit `ConnectionEvent.RetryScheduled(attempt = ++count, nextRetryAtMs = clock() + delayMs)`. -- `ConnectionReducer.RetryScheduled` already handles state; `Connected` resets attempt to 0. -- `StatusBar` formats `nextRetryAtMs - clock()` as countdown. - ---- - -## Pending — resume here -### Task 4.1 — Scrollable session output log -- Extend `SessionsState`: `eventLog: Map>`, `scrollOffsetById: Map`. -- New `LogEntry(timestampMs, kind, text)`. -- `SessionsReducer`: on `ServerEventReceived` for InferenceCompleted/ToolCompleted/StageStarted/StageCompleted/StageFailed, append entry (cap 200). -- Rename `ActiveSession.kt` → `SessionOutputLog.kt`; render last N entries with scroll offset. -- Add `Action.ScrollLogUp/Down`; introduce `FocusState { SessionList, OutputLog }`, Tab toggles. Map `NavUp/NavDown` to scroll when focus is on log. - -### Task 5.1 — Coverage gate -- Add kover verify rule to `apps/tui/build.gradle`: minBound 70 on `com.correx.apps.tui.reducer.*` + `com.correx.apps.tui.input.*`. Confirm syntax against current kover version. - -### Task 5.2 — Final cleanup pass -- Audit every `@Suppress` under `apps/tui/src/main`; either fix or document. -- Update `docs/refactor.md` 2026-05-17 entries to `[done]`. - ---- - -## Sequencing -``` -[done] 1.1 → 1.2 → 1.3 → 1.4 → 2.1 → 2.2 → 2.3 → 3.1 → 3.2 → 3.3 → {4.2, 4.3} -[next] 4.1 → 5.1 → 5.2 -``` - -## Renderer evaluation — Mosaic is the floor - -Mosaic (jake-wharton/mosaic 0.18) is intentionally minimalist: re-emits plain text lines per frame, no screen-cell buffer, no widget catalog, no built-in borders/modals/scroll regions/mouse, no focus management. We hand-draw `─├┤` in `components/Panel.kt`. That is the entire framing system. - -This is fine for a reducer/effect prototype but it caps how polished the TUI can ever look. Compared to Python's Textual we lack: cell-buffer rendering, layout engine, widget library (DataTable / Tree / Input / Modal), focus + mouse, scroll regions, transitions. - -**Candidate replacements to evaluate (renderer swap; reducer/Action/Effect core stays):** -- **tamboui** — user-suggested: https://github.com/tamboui/tamboui (not investigated yet — no web access this session). -- **Kotter** — Compose-style DSL on JVM, more capable than Mosaic (real text-attribute model, animations, structured input). Closest semantic match to current code. -- **Lanterna** — Java TUI lib with real `TerminalScreen` buffer, GUI widgets (`Panel`, `Window`, `Table`, `TextBox`), borders, layouts, mouse. Imperative/Swing-style; bigger rewrite of `TuiApp` + `components/` but reducers untouched. -- **Mordant** — already transitive; only buys nicer ANSI paint, same ceiling as Mosaic. - -**Migration cost:** ~all of `apps/tui/src/main/.../TuiApp.kt` and `components/*`. Unchanged: `input/`, `reducer/`, `state/`, `ws/` — the renderer-agnostic core. That's the architectural payoff of Task 2.2/2.3. - -**Decision:** deferred. Capture as standalone spec before swapping. - -## Notes for the next session -- Working tree currently has no commits since `58670a6`; everything is staged-or-modified. Consider committing per task on resume. -- Kotlin is now 2.2.10 repo-wide; expect occasional `experimental context receivers` warnings (`-Xcontext-receivers`) — future Kotlin will error on this. -- Mosaic 0.18 `KeyEvent` lives at `com.jakewharton.mosaic.layout.KeyEvent` with fields `key: String, alt, ctrl, shift`. `onKeyEvent` is `com.jakewharton.mosaic.layout.onKeyEvent`. -- All `EventPayload` subclasses must be registered in `core/events/.../serialization/Serialization.kt` — relevant for Task 4.1 if any new event types are added (none currently planned). diff --git a/qa/audit-report-2026-06-08.md b/qa/audit-report-2026-06-08.md index ac7b4905..ee751cbe 100644 --- a/qa/audit-report-2026-06-08.md +++ b/qa/audit-report-2026-06-08.md @@ -52,7 +52,8 @@ Patches applied this session (rebuild + restart server to load): - F-020: `FileEditTool` — model-correctable failures (bad patch, target-not-found, bad params, unknown op) now `recoverable=true` (feed back, don't abort); tool/`operation` descriptions + implementer prompt steer toward `replace`/`file_write` over the fragile `patch` op. -- F-021: OPEN (not patched) — resume `rehydrate()` slot-name-vs-CAS-hash bug; see finding. +- F-021: FIXED — resume `rehydrate()` now consumes `ArtifactContentStoredEvent` (slot→hash) + and fetches CAS bytes by content hash instead of passing the slot name to the hash-keyed store. **Key reproducibility note:** the live config the server reads is `~/.config/correx/` (`configDir = ConfigLoader.configPath().parent`), NOT the repo. Code fixes load via repo @@ -426,10 +427,13 @@ repo `docs/schemas/` were stale relative to live — a clean-checkout reproducti - **fix direction:** rehydrate from `ArtifactContentStoredEvent` — `artifactStore.get(contentHash)` then `artifactContentCache["sessionId:slot"] = content`. The F-007/F-015 events already record the needed slot→hash mapping; this just consumes it on cold start. -- **status:** OPEN (found by code inspection while answering "is resume actually implemented?"). - Replay (`ReplayOrchestrator` + `testing/replay/` suite) is implemented and unit-tested; resume's - wiring exists (`POST /sessions/{id}/resume`, `resume()`, `rehydrate()`) but this bug means it - almost certainly does not restore content correctly — needs a live #1/#8 probe to confirm. +- **status:** FIXED — `rehydrate` now scans `ArtifactContentStoredEvent` (the durable slot→CAS-hash + bridge added in F-007) and calls `artifactStore.get(p.contentHash)`, keying the cache by the + logical slot name (`p.artifactId`). The slot name is never passed to the hash-keyed CAS, so the + odd-length-hex crash is gone and content is restored on cold start. `RehydrateTest` reworked to + use a content-addressed fake store (slot name ≠ content hash) so it actually exercises the bug; + both cases green. Replay (#8) unaffected — rehydrate is live-resume-only. Still wants a live + #1/#8 resume probe to confirm end-to-end restoration. ### Steering (functional contract) — partially exercised live - During the live operator sessions, approval-with-steering was informally validated: the operator diff --git a/testing/integration/src/test/kotlin/RehydrateTest.kt b/testing/integration/src/test/kotlin/RehydrateTest.kt index 22b1edef..133dac68 100644 --- a/testing/integration/src/test/kotlin/RehydrateTest.kt +++ b/testing/integration/src/test/kotlin/RehydrateTest.kt @@ -4,7 +4,7 @@ import com.correx.core.approvals.DefaultApprovalRepository import com.correx.core.approvals.domain.DefaultApprovalEngine import com.correx.core.artifactstore.ArtifactStore import com.correx.core.artifacts.DefaultArtifactReducer -import com.correx.core.events.events.ArtifactValidatedEvent +import com.correx.core.events.events.ArtifactContentStoredEvent import com.correx.core.events.events.EventMetadata import com.correx.core.events.events.NewEvent import com.correx.core.events.types.ArtifactId @@ -44,15 +44,21 @@ import java.util.UUID class RehydrateTest { private val sessionId: SessionId = TypeId("session-rehydrate") - private val artifactId: ArtifactId = TypeId("artifact-001") + + // The CAS is content-addressed: the stored key is a content hash, NOT the + // logical slot name. F-021 — rehydrate must resolve slot.name -> contentHash + // via ArtifactContentStoredEvent and fetch by hash, never pass the slot name + // (which is non-hex and would crash a real CAS) to ArtifactStore.get. + private val slotName: ArtifactId = TypeId("analysis") + private val contentHash: ArtifactId = TypeId("a1b2c3d4") private val artifactContent = """{"status":"success"}""" private val eventStore = InMemoryEventStore() private val fakeArtifactStore: ArtifactStore = object : ArtifactStore { - override suspend fun put(bytes: ByteArray): ArtifactId = artifactId + override suspend fun put(bytes: ByteArray): ArtifactId = contentHash override suspend fun get(id: ArtifactId): ByteArray? = - if (id == artifactId) artifactContent.toByteArray(Charsets.UTF_8) else null + if (id == contentHash) artifactContent.toByteArray(Charsets.UTF_8) else null override suspend fun flushBefore(commit: suspend () -> Unit) = commit() } @@ -100,7 +106,7 @@ class RehydrateTest { decisionJournalRepository = decisionJournalRepository, ) - private suspend fun appendValidatedEvent() { + private suspend fun appendContentStoredEvent() { eventStore.append( NewEvent( metadata = EventMetadata( @@ -111,8 +117,9 @@ class RehydrateTest { causationId = null, correlationId = null, ), - payload = ArtifactValidatedEvent( - artifactId = artifactId, + payload = ArtifactContentStoredEvent( + artifactId = slotName, + contentHash = contentHash, sessionId = sessionId, stageId = StageId("stage-1"), ), @@ -121,20 +128,21 @@ class RehydrateTest { } @Test - fun `rehydrate populates artifactContentCache from ArtifactValidatedEvent`(): Unit = runBlocking { - appendValidatedEvent() + fun `rehydrate resolves slot name to content hash and populates cache`(): Unit = runBlocking { + appendContentStoredEvent() orchestrator.rehydrate(sessionId) - val cached = orchestrator.validatedArtifactContent(sessionId, artifactId) + // Cache is keyed by the logical slot name; content fetched from CAS by hash. + val cached = orchestrator.validatedArtifactContent(sessionId, slotName) assertEquals(artifactContent, cached) } @Test - fun `rehydrate is a no-op when there are no ArtifactValidatedEvents`(): Unit = runBlocking { + fun `rehydrate is a no-op when there are no ArtifactContentStoredEvents`(): Unit = runBlocking { orchestrator.rehydrate(sessionId) - val cached = orchestrator.validatedArtifactContent(sessionId, artifactId) + val cached = orchestrator.validatedArtifactContent(sessionId, slotName) assertEquals(null, cached) } }