Skip to content

♻️ refactor: ADR-023 S5-5 — code-merge and close-out (#1685) - #1688

Open
tyabu12 wants to merge 7 commits into
mainfrom
refactor/kmp-s5-5-code-merge
Open

♻️ refactor: ADR-023 S5-5 — code-merge and close-out (#1685)#1688
tyabu12 wants to merge 7 commits into
mainfrom
refactor/kmp-s5-5-code-merge

Conversation

@tyabu12

@tyabu12 tyabu12 commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Summary

ADR-023 Stage 5 close-out. Decision 6 fires: the Kotlin engine becomes the sole fresh-run path; the Swift SimulationRunner stays for resume() only (Kotlin exports no resume-from-state).

  • TurnOutput.rawText ported to Kotlin (body @Transient var, outside equals/copy/wire shape like the Swift twin; set by JSONResponseParser, re-set by WhisperHandler after copy()), so TurnRecord.rawOutput is populated on the Kotlin path instead of dropping the audit column (which would need a GRDB migration on a shipped DB).
  • S5-3 H7 crash probe deleted (Kotlin H7CrashProbe + inverse @Throws pin, H7CrashTrigger, Settings reveal gesture / alert / button, h7CrashProbeEnabled, catalog keys, kmp-interop Pattern 5 carve-out). Lands before the next App Store submission, per the ADR-023 §6 S5-5 ordering constraint.
  • Default flipped: SimulationViewModel drops isSharedEngineEnabled; makeSharedRunner is a defaulted non-optional factory. The YAML-less branch survives only as the test seam ~66 existing run(scenario:) suites use, loud (assertionFailure off the test harness + .error log) and tracked by ♻️ refactor: migrate YAML-less SimulationViewModel test call sites to the Kotlin engine #1687.
  • S5-4 switch surface deleted (11 sites, including PasturaApp.clearSharedEngineOptInOffSandbox(), which the ADR list missed and which would have wiped the new default on every production launch); BuildChannel goes with it. SettingsView+Diagnostics.swift → SettingsView+About.swift.
  • Gate spike retired (tools/kmp-gate-spike/, −3.8k lines): B′ guard re-homed to scripts/kmp/check-b-prime-isolation.sh under the ruleset-frozen kmp-gate-isolation job name; suspendcontroller-drift job deleted; nightly Pattern 7 probe stages the app-tree slice; duplicate-basename-gate.sh A5 plants its pairs in a temporary index. The macosArm64 parity rung is untouched.
  • Docs: ADR-023 §16 (Decision 6 fired; two in-place factual corrections to the S5-5 bullet — no Engine/ file was ever deletable, and the Kotlin engine becomes the sole fresh-run path), ADR-004 §9.7 closes (§13), status board / ROADMAP / INDEX / CLAUDE.md § Current Phase drop the Stage-5 priority track.

Repository ruleset already updated (operator-approved in session): SuspendController verbatim-copy drift guard removed from the required status checks; KMP gate-spike isolation guard (B′) kept.

Accepted residuals: #1687 (test-seam migration; make yamlDefinition: non-optional). No Swift fallback for a YAML the Kotlin loader rejects — LoaderAcceptanceParityTests pins Kotlin acceptance of every preset / demo preset / gallery seed / serializer round-trip the Swift loader accepts.

Dates in the docs assume a 2026-09-07 merge; adjust if it lands later.

Test plan

  • ./gradlew :shared:models:jvmTest :shared:engine:jvmTest + macosArm64Test (rawText slice)
  • scripts/kmp/assemble-xcframework.sh — verifyExportedThrowsAnnotations forward pin only, H7CrashProbe absent from the umbrella header
  • scripts/kmp/probe-pattern7-isolation.sh — PASS on the default slice (no PASTURA_PROBE_SLICE_DIR)
  • scripts/xcodebuild.sh test -skip-testing:PasturaUITests — 3713 tests / 301 suites green after the flip (the YAML-less suites still pass through the seam)
  • swiftlint lint --quiet --strict — clean
  • swift build (root/harness) — green; bash scripts/tests/kmp-gate-isolation-test.sh 23/23; bash scripts/duplicate-basename-gate.sh --self-test 9/9 (A5 and A8 still redden under their mutations)
  • CI: kmp-gate-isolation reports under its unchanged name; nightly Pattern 7 step green on the first run after merge
  • Skipped: PasturaUITests (no UI test references the removed identifiers; settings.versionRow survives).

Review

Reviewer: Opus (code-reviewer, seven slices — Kotlin models / engine, App/KMP + VM, Views + catalog, tests, CI + scripts, docs). All seven PASS, 0 Critical. Applied 25 findings in f096dd6b (loader-parity floors count comparisons performed; rawText write-once-before-emit invariant stated as the kmp-interop Pattern 1 carve-out; stale comments and docs).

Rejected:

  • Drop yamlDefinition:'s = nil default now — deferred to ♻️ refactor: migrate YAML-less SimulationViewModel test call sites to the Kotlin engine #1687 (66 test call sites).
  • A8 fixture root under tools/harness is a root-manifest target — accepted with the rewritten rationale; the fixtures are empty and trap-cleaned.
  • SimulationViewModelSharedEngineTests replayed-pause wall-clock bound — pre-existing, out of scope.
  • nonisolated private static ordering — SwiftLint modifier_order enforces the current form.

Device QA

Required — the diff changes the shipping engine for every fresh run and the K/N bridge (rawText), which the simulator exercises only via Ollama:

  1. Fresh run of target_score_race and word_wolf on the Kotlin engine (no toggle exists any more) — streaming rows show extracted statement text; run completes.
  2. Open the result detail — turn rows render; rawOutput is non-empty for a Kotlin turn (Past Results storage size no longer reads ~0 for new runs).
  3. Pause → resume and kill → resume-via-Swift-runner cycles behave as in the S5-4 soak.
  4. Settings: About section shows the version row; no Diagnostics section, no 5-tap gesture.

Context-economy: rule-file edits are net −7 lines (kmp-interop.md +20/−27, others ±1); every added paragraph replaces retired gate-spike guidance (twin rule → PasturaTests/App/KMP/, Pattern 1 write-once carve-out, Pattern 5 carve-out removed) — kept, nothing new to drop.

Closes #1685

🤖 Generated with Claude Code

https://claude.ai/code/session_019i4vDq51ugsZ2Ror8xz5Ph

tyabu12 and others added 7 commits September 6, 2026 23:59
@tyabu12 @claude
✨ feat: port TurnOutput.rawText to the Kotlin engine (S5-5)
7885686 
The Kotlin run path forced `rawText: nil` at the bridge, leaving the
`TurnRecord.rawOutput` audit column empty for every Kotlin-engine turn
(ADR-023 §6 S5-4 residual). Port the field instead of dropping the
column: a body-declared `@Transient var` keeps it outside `equals` /
`copy` / the wire shape exactly as the Swift twin excludes it from `==`
and `Codable`, and avoids a GRDB migration on a shipped DB.

`JSONResponseParser` sets it from the pre-cleanup text; `WhisperHandler`
re-sets it after its `copy()` (body properties do not survive `copy`).

Part of #1685

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019i4vDq51ugsZ2Ror8xz5Ph
@tyabu12 @claude
🔥 chore: delete the S5-3 H7 crash probe (S5-5)
9cfacd3 
Decision 6 (ii) is discharged (ADR-004 §11), so the probe's discovery
role is over and its reveal gesture must not reach App Review
(ADR-023 §6 S5-5 ordering constraint). Removes the Kotlin
`H7CrashProbe` + test and the inverse `verifyExportedThrowsAnnotations`
pin, `H7CrashTrigger`, the Settings reveal gesture / confirmation /
crash button and their three `@State`s, `FeatureFlags.h7CrashProbeEnabled`,
four catalog keys, and the kmp-interop Pattern 5 carve-out.

`Diagnostics` (the section header) stays until the S5-4 switch surface
goes; the section is now gated on `isSandboxOrDebug` alone.

Part of #1685

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019i4vDq51ugsZ2Ror8xz5Ph
@tyabu12 @claude
✨ feat: Kotlin engine becomes the sole fresh-run path (S5-5)
7fca775 
Decision 6 (i)–(iii) are discharged (ADR-023 §5 D1), so the S5-4
opt-in gate goes: `SimulationViewModel` drops `isSharedEngineEnabled`
and builds a `SharedEngineRunner` by default for every fresh run that
carries the scenario YAML. `resume()` stays on the Swift `SimulationRunner`
(Kotlin exports no resume-from-state).

The YAML-less branch survives only as the test seam ~66 existing
`run(scenario:)` suites use — loud (`assertionFailure` off the test
harness, `.error` log always) and tracked by #1687.

Adds a differential loader-acceptance test: every YAML the Swift loader
accepts (presets, demo presets, gallery seeds, serializer round-trips)
must load on the Kotlin loader too, since there is no Swift fallback
and a divergence would surface only at run time.

Part of #1685

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019i4vDq51ugsZ2Ror8xz5Ph
@tyabu12 @claude
🔥 chore: delete the S5-4 opt-in engine switch (S5-5)
f8f02c3 
The Kotlin engine is now the sole fresh-run path, so the S5-4 opt-in
surface has no reader: `FeatureFlags.sharedEngineEnabled`, the
`PasturaApp` opt-in eraser (`clearSharedEngineOptInOffSandbox`, which
would otherwise have wiped the default on every production launch), the
Settings Diagnostics section (toggle + `ja` sample row),
`SharedEngineDiagnostics`, `BuildChannel` (no adopter survives), and the
four catalog keys. `SettingsView+Diagnostics.swift` becomes
`SettingsView+About.swift` — only the About section remains.

`docs/qa/kmp-soak-qa.md` is rewritten for a default-on engine.

Part of #1685

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019i4vDq51ugsZ2Ror8xz5Ph
@tyabu12 @claude
🔥 chore: retire the Stage-2 gate spike (S5-5)
99b76d5 
The iOS app is now the real consumer of `PasturaSharedEngine`, and its
`PasturaTests/App/KMP/` suite exercises the boundary the spike's
BoundaryContract / Pattern-6 probes covered. The `macosArm64` parity
rung (Stage 4: `EngineParityTests` + `tools/harness`) is independent
and untouched.

Every non-parity asset is re-homed before the deletion:
- B′ isolation guard → `scripts/kmp/check-b-prime-isolation.sh`; the
  `kmp-gate-isolation` job keeps its ruleset-frozen name, check (2)
  ("root manifest references the spike") is moot and dropped.
- `suspendcontroller-drift` job deleted (its verbatim copy dies with the
  spike; the ruleset no longer requires it).
- Nightly: spike build/test and gate-bench steps gone; the Pattern 7
  probe stages the app-tree slice via `assemble-xcframework.sh
  --if-missing` as its own S5-5 note prescribed.
- `duplicate-basename-gate.sh`: spike rows dropped; A8 fixture root →
  `tools/harness`; A5 plants its two cross-target pairs in a temporary
  index (`GIT_INDEX_FILE`) now that the spike's were the last real ones.
- `.claude/rules` twin rule: adapter twins live in `PasturaTests/App/KMP/`.

Part of #1685

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019i4vDq51ugsZ2Ror8xz5Ph
@tyabu12 @claude
📝 docs: ADR-023 S5-5 close-out — Decision 6 fired, Stage 5 closed
f94e247 
ADR-023 §16 records the landing (default flipped, probe + switch
deleted, gate spike retired, rawText ported), the two accepted
residuals (#1687 test seam; no Swift fallback for Kotlin-rejected
YAML), and two in-place factual corrections to the S5-5 bullet: no
`Engine/` file was ever deletable (the `REPLACED` rows are shared
`LLM/` types; `resume()` and the Stage-4 harness need `SimulationRunner`),
and the Kotlin — not Swift — engine becomes the sole fresh-run path.
ADR-004 §9.7 closes (§13). Status board, ROADMAP, INDEX and CLAUDE.md
§ Current Phase drop the Stage-5 priority track.

Dates use 2026-09-07; adjust to the merge date if the PR lands later.

Part of #1685

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019i4vDq51ugsZ2Ror8xz5Ph
@tyabu12 @claude
🐛 fix: S5-5 review follow-ups — loader parity floors, stale rawText n…
f096dd6 
…otes

Review findings applied across the branch:
- `LoaderAcceptanceParityTests` counts comparisons performed (not files
  enumerated), keys its dedupe on the app bundle first, and pins the
  real corpus sizes so the suite cannot go vacuous.
- `rawText`'s write-once-before-emit invariant is stated on the Kotlin
  property, at the Swift bridge, and as the Pattern 1 carve-out in
  `kmp-interop.md` (a Kotlin `var` now crosses the retroactive
  `Sendable` conformance).
- Stale comments corrected: parity `normalize` / `EventLineMapper`
  rationale, `run(...)` doc on `yamlDefinition:`, the UI-test caveat on
  the test-harness probe, `@Throws` pin count (seven), nightly timing
  terms, README `shared/` tree, ADR-004 §12 spent marker, ADR-023 S5-1
  guard checks, `h7-symbolication-qa.md` marked historical.

Part of #1685

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019i4vDq51ugsZ2Ror8xz5Ph
@tyabu12 tyabu12 added the refactor Code refactoring label Sep 6, 2026
@tyabu12 tyabu12 self-assigned this Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

CI Report

Lint

SwiftLint: passed

Release Build (ADR-005 §8 guard)

Release-iphoneos symbol guard: passed

Demo Replay Drift Guard

Demo replay drift guard: passed

Test Results

  • Unit: > Could not parse test output.
  • UI: > Could not parse test output.

Coverage

73.6% line coverage

View full CI run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@tyabu12 tyabu12

Labels

refactor Code refactoring

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

♻️ refactor: ADR-023 S5-5 — code-merge and close-out

1 participant

@tyabu12