Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ releases could contain breaking changes when called out explicitly.

### Fixed

- Keep conversation compilation previews recoverable after lock-contention and filesystem errors, returning an error reply and preserving pending actions instead of failing the turn.

- Preserve affected artifact references such as `go.mod` in conversation previews without treating them as source-model locations.

- Advertise conversation protocol support in the actual LSP initialize response so native editor chat can pass capability negotiation.
Expand Down
11 changes: 10 additions & 1 deletion cli/src/modelable/llm/filesystem_conversation.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ def preview_compilation(
CompilationPolicy,
CompilationRequest,
)
from modelable.operations.file_transaction import LockContentionError
from modelable.registry.ids import RegistryIdLockContentionError

replaced = self._pending
actual_replaced_id = _pending_id(replaced)
Expand All @@ -182,7 +184,14 @@ def preview_compilation(
),
policy=CompilationPolicy.conversation(),
)
except (CompilationError, json.JSONDecodeError, sqlite3.OperationalError) as error:
except (
CompilationError,
json.JSONDecodeError,
sqlite3.OperationalError,
LockContentionError,
RegistryIdLockContentionError,
OSError,
) as error:
return ConversationReply(
kind="error",
text=f"Could not preview compilation: {_escape_inline(error)}",
Expand Down
20 changes: 12 additions & 8 deletions cli/tests/test_conversation_preview_recovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@

from modelable.llm.conversation import ConversationSession
from modelable.operations.compilation import CompilationError
from modelable.operations.file_transaction import LockContentionError
from modelable.registry.ids import RegistryIdLockContentionError

OPERATIONAL_ERRORS = [
json.JSONDecodeError("bad ledger", "{", 1),
sqlite3.OperationalError("database unavailable"),
RegistryIdLockContentionError("ledger contention"),
LockContentionError("publication contention"),
OSError("staging allocation failure"),
]


@pytest.fixture
Expand All @@ -20,9 +30,7 @@ def preview_session(catalog_workspace):
session.close()


@pytest.mark.parametrize(
"error", [json.JSONDecodeError("bad ledger", "{", 1), sqlite3.OperationalError("database unavailable")]
)
@pytest.mark.parametrize("error", OPERATIONAL_ERRORS)
def test_first_preview_operational_failure_is_a_domain_error(preview_session, monkeypatch, error):
session, root, stages, request, service = preview_session
original = request.source.read_bytes()
Expand Down Expand Up @@ -52,11 +60,7 @@ def test_preview_programming_faults_still_propagate(preview_session, monkeypatch

@pytest.mark.parametrize(
"error",
[
CompilationError("preview rejected"),
json.JSONDecodeError("bad ledger", "{", 1),
sqlite3.OperationalError("database unavailable"),
],
[CompilationError("preview rejected"), *OPERATIONAL_ERRORS],
)
def test_failed_replacement_keeps_pending_stage_until_close(preview_session, monkeypatch, error):
session, root, stages, _request, service = preview_session
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/test_offline_feature_qualification.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from modelable.emitters.targets import list_implemented_codegen_targets

RUST_FIXTURE = Path(__file__).parent / "fixtures" / "offline-rust-consumer"
FAST_GENERATION_BUDGET_SECONDS = 30
FAST_GENERATION_BUDGET_SECONDS = 45
LANGUAGE_MATRIX_BUDGET_SECONDS = 120

PRODUCER_V1 = """
Expand Down
5 changes: 5 additions & 0 deletions docs/superpowers/specs/system-spec/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
- Historical intake: 2026-10-31. Product baseline: `0fead2321bdef11c72b2746eb57f3a8b0e327c94` (Modelable 1.15.1).
- Historical metamodel: system-specification `00188b037797fd37240dab9a0a11c431815e3ac9`; original schema retained as [schema-baseline.json](schema-baseline.json), original hashes/environment in [baseline.json](baseline.json). The manifest describes the original files at Modelable commit `5f48ed00`, not the migrated compilation/schema files.
- Compilation characterization: Modelable `72de182a47e1c045f586be73a2c3a79b68f9130b` (1.15.2), capture validator `b187e43628c7dc89935d17c2c382094ad37eb2eb`. Current [schema.json](schema.json), pins/hashes in [adoption.json](adoption.json), and [current recovery checks](evidence/recovery-validation.json). Prior captures remain historical.
- Whole-product Level 3 **readiness record** (not accepted; release stage fails): [whole-product-level3.md](whole-product-level3.md).
- Whole-product Level 2 **draft** (not accepted): five pinned behaviors with evidence in [evidence/level2](evidence/level2), [decision record](whole-product-level2.md); validates at `verification`.
- Whole-product Level 1 **draft** (not accepted): [whole-product.yaml](whole-product.yaml), [decision record](whole-product-level1.md).
- Status: whole-product inventory recorded; compilation behavior pinned for the bounded scope below, with explicit source/version and artifact bindings. Whole-product Levels 1–3 remain open.
- Live onboarding TODO: inventory and bounded compilation discovery recorded; [dependency/state map](compilation-map.md) covers 141 source modules and 19 declared dataclass shapes. The two missing symlink cases now pass on Linux. Shared IR/schema semantics, public failure catalog and remaining recovery/transport coverage, classification, quality criteria, formal assessment and release review remain open.
- This is internal development metadata. Product authority remains [architecture.md](../../../architecture.md), the existing capability catalog and native acceptance gates. The metadata changes no language, dependencies, installed skills or product commands. The accompanying native-gate repair moves graph controls above the canvas and makes browser characterization wait for the requested layout.
Expand Down Expand Up @@ -92,6 +95,8 @@ system-spec --version
system-spec docs/superpowers/specs/system-spec/inventory.yaml --stage inventory --check-artifacts --root .
system-spec docs/superpowers/specs/system-spec/compilation.yaml --stage verification --check-artifacts --root .
system-spec docs/superpowers/specs/system-spec/compilation-contract-proposal.yaml --stage architecture --check-artifacts --root .
system-spec docs/superpowers/specs/system-spec/whole-product.yaml --stage architecture --check-artifacts --root .
system-spec docs/superpowers/specs/system-spec/whole-product.yaml --stage verification --check-artifacts --root .
system-spec docs/superpowers/specs/system-spec/compilation.yaml --context compilation-unit
system-spec docs/superpowers/specs/system-spec/compilation.yaml --impact apply-compilation
system-spec docs/superpowers/specs/system-spec/inventory.yaml --plan local-contract-toolchain
Expand Down
47 changes: 47 additions & 0 deletions docs/superpowers/specs/system-spec/browser-persistence-map.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Browser persistence map (bounded)

- Source: branch `fix/conversation-preview-operational-errors` at `e78a03be`; read from `web/src/{workspace-repository,usePersistentWorkspace,workspace,theme,sw-registration}.ts(x)`, `web/src/App.tsx` (repository wiring), `web/vite.config.ts` and `docs/playground-design.md` §15–16. Observed behavior only; no tests or browser runs were performed for this map.
- Authority: the existing playground design doc, source and native gates. This extends the [inventory](inventory.yaml) "Playground" row; it changes no contract.
- **Not mapped:** the Pyodide worker's in-memory state, the AI/WebLLM session and model-download state (`web/src/ai/**`), and the browser DTO/protocol schemas. Their lifetimes remain open.

## Persistent stores

| Store | Owner | Contents / lifetime |
|---|---|---|
| IndexedDB `modelable-playground`, version 1, object store `workspaces` (keyPath `id`) | `IndexedDbWorkspaceRepository` | One `PlaygroundWorkspace` per id: `schemaVersion` (1), `id`, `revision`, `files[] {path, content, version}`, `activeFile`, optional `metadata.lastAiAccept {provider, model, timestamp}`. Source of truth for user source; survives reloads. |
| `localStorage` key `modelable:theme` | `theme.ts` | `'light'` or `'dark'`; absence means `system`. Reads/writes are wrapped in try/catch (storage may be disabled). |
| Service worker + Workbox precache | `vite-plugin-pwa` in `vite.config.ts` | `registerType: 'autoUpdate'`, scope `/modelable/playground/`; precaches `js, css, html, wasm, mjs, zip, json, whl, ttf, woff, woff2, png` (excluding `fixtures/**`, files up to 30 MB), navigate fallback to `index.html`. Holds app and Pyodide assets, not user data. |

Derived results (compiled artifacts, diagnostics, graph) are memory-only; the design doc says the same (§ around line 1041).

## Repository protocol

| Operation | Observed behavior |
|---|---|
| `load(id)` | Readonly transaction. `missing` if absent; `ready` with the parsed workspace; `recovery-required` (`invalid` or `incompatible`, carrying the raw record) when `parseWorkspaceRecord` throws `WorkspaceValidationError`. Other errors propagate. Unsupported `schemaVersion` -> `incompatible`. |
| `save(workspace)` | Readwrite transaction; reads the stored record and returns `'stale'` without writing only if the stored `revision` is **greater** than the one being saved. Equal or lower stored revision is overwritten. Writes a `structuredClone`. |
| `remove(id)` | Deletes the record. |
| Schema upgrade | `onupgradeneeded` creates the `workspaces` store if missing. There is no migration path beyond version 1; a different stored `schemaVersion` is surfaced for recovery rather than migrated. |

## Hook protocol (`usePersistentWorkspace`)

- Phases: `restoring`, `saving`, `saved`, `memory-only`, `recovery-required`.
- On mount: `load(default id)`; `missing` -> adopt the default workspace and save immediately; `recovery-required` -> hold the raw record and block normal editing; load rejection -> `memory-only`.
- Edits: set dirty, debounce 300 ms, then save; a monotonic sequence number and the current revision gate the `saved` transition so a late older save cannot mark a newer edit as saved.
- `pagehide` with unsaved changes triggers an un-awaited save.
- Save rejection (including IndexedDB unavailable) -> `memory-only`; no automatic retry was found.

## Observations for follow-up (from reading source, not confirmed by tests)

- **`'stale'` result is ignored.** The hook awaits `repository.save(snapshot)` without inspecting its return value, so a `'stale'` result is treated the same as `'saved'` (phase becomes `saved` if the in-memory revision still matches). With two tabs on the same workspace id, the lower-revision tab's save is rejected by the repository but shown as saved. No test in `usePersistentWorkspace.test.tsx` references a `'stale'` save result (grep); repository-level behavior is tested in `workspace-repository.test.ts`. Maintainers should decide the intended UI state before treating this as a defect.
- **Equal-revision overwrite.** Two tabs at the same revision overwrite each other; there is no cross-tab notification (grep found no `BroadcastChannel` or `storage` listener in `web/src`).
- **Unmount drops a pending debounce.** The effect cleanup clears the timer without flushing; only `pagehide` flushes. An unmount that is not a page unload would lose the last debounced edit.
- **`pagehide` save is best effort.** The IndexedDB write is not awaited by the browser lifecycle, so completion is not guaranteed.
- **Design-doc drift.** `docs/playground-design.md` lists IndexedDB stores for editor state, cached compiler metadata, graph layouts and model download state; the only store in code is `workspaces`. Record as an as-is discrepancy rather than a missing requirement until maintainers confirm which is authoritative.
- **Service-worker update safety.** The design doc requires that an update must not silently invalidate unsaved state; the config uses `autoUpdate`. Whether an update reloads the page while the workspace is dirty was not verified here.
- **Secrets.** `metadata.lastAiAccept` stores provider/model names only; the design doc forbids secrets in IndexedDB/`localStorage`. No secret storage was found in the files read, but `web/src/ai/**` was not searched exhaustively beyond a storage-API grep, which found none.

## Disposition

- Accepted as a bounded observed map of workspace persistence. Classification of stored data remains open: workspace content is user source text and should be treated as at least the compilation DTOs' provisional `restricted` handling; this is not an observed product classification.
- Candidate follow-ups for maintainers to schedule: a test and decision for handling `'stale'` saves; a decision on cross-tab behavior; confirmation of the service-worker update/dirty-state behavior; reconciling the design doc's store list with the code.
3 changes: 3 additions & 0 deletions docs/superpowers/specs/system-spec/compilation-map.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ Direct and conversational policies differ. Direct path mapping preserves the exi
- Compensation restores backups/removes newly written files where possible. Errors and preserved backups are reported. Cleanup can leave quarantined `.modelable-cleanup-*` files or released-lock artifacts; they are operational recovery state, not accepted product output.
- These are multiple per-file replacements with compensation. No durable multi-file commit journal or crash-atomicity proof is asserted. PID-based stale-lock reclamation is observed behavior, not formal assurance against every crash/race scenario.
- `LockContentionError` and `RegistryIdLockContentionError`, plus lower-level filesystem failures, can cross helper boundaries beyond the short service exception list in the original specification. Complete adapter translation/error-catalog review remains under MOD-COMP-09; this map does not silently redefine the frozen contract or claim those cases are covered by the original characterization record.
- Conversation preview (`filesystem_conversation.py`) now returns a `kind="error"` reply for `LockContentionError`, `RegistryIdLockContentionError` and `OSError`, keeping any pending action; characterized in `cli/tests/test_conversation_preview_recovery.py`. Apply already caught all exceptions. The service contract and other adapters (LSP wire, editor) are unchanged; this is not a full error-catalog closure.
- Formal applicability remains open for lock ordering, freshness under contention, rollback and crash windows. Existing unit tests are supporting behavior evidence; this source map is not a proof.

## Symlink coverage decision
Expand All @@ -81,4 +82,6 @@ To reproduce, create a fresh temporary build context, extract `git -c core.autoc
- Archive the merged [migration plan](../../plans/archived/system-spec-evidence-migration.md); the ongoing onboarding specification stays active.
- Accept this bounded observed dependency/state map and the suitable-host execution of MOD-COMP-10 as supporting evidence. Preserve the original capability/data contracts and the existing category waiver.
- Keep whole-product architecture/release open. MOD-COMP-09 still owns measured performance/scalability/observability requirements, complete public error/failure review, transitive IR/schema semantics and formal applicability. Provisional classification review is also open.
- Browser workspace persistence is mapped in [browser-persistence-map.md](browser-persistence-map.md).
- Registry transport, ledger and lock protocols are mapped in [registry-protocol-map.md](registry-protocol-map.md).
- Next bounded review: reconcile the public error catalog and lock/compensation obligations, then map shared registry protocols and browser persistence against their existing authorities. Maintainers own acceptance and remediation scheduling.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
SPECIFICATION Spec
CONSTANTS RollbackMayFail = FALSE CanCrash = FALSE
INVARIANTS PromotionExclusive LocksHeldByActive FilesConsistent NoLostUpdate
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
SPECIFICATION Spec
CONSTANTS RollbackMayFail = TRUE CanCrash = FALSE
INVARIANTS FilesConsistent
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
SPECIFICATION Spec
CONSTANTS RollbackMayFail = FALSE CanCrash = TRUE
INVARIANTS FilesConsistent
Loading
Loading