Skip to content

Commit dc09d6f

Browse files
committed
feat: Differentiate between kernel running and kernel compiling for compile mode.
1 parent c469974 commit dc09d6f

15 files changed

Lines changed: 214 additions & 21 deletions

File tree

cli-skill/plugins/siza/skills/siza/SKILL.md

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ The user has a Sabela notebook open in their browser. You are pairing with them:
1212

1313
**Every notebook operation goes through `${CLAUDE_PLUGIN_ROOT}/skills/siza/scripts/siza-tool.sh`.** Do not `curl` `/api/cell/*`, `/api/load`, `/api/notebook`, or any non-`/api/ai/*` endpoint, and do not `ps aux` looking for the server. Raw endpoints bypass the AI bridge: they skip the browser-refresh broadcast, skip optimistic-concurrency checks on cell hashes, and skip large-output handle stashing. If `siza-tool.sh` doesn't expose what you need, tell the user — don't reach around it. **One sanctioned exception:** setting widget state (slider/dropdown/lasso) via `POST /api/widget`, since siza has no widget-set tool — see [Driving widgets](#driving-widgets) below. It still needs user sign-off before you `curl` it.
1414

15+
**Never edit the notebook's `.md` file directly with file tools (Write/Edit/sed).** The live session the user is looking at is the source of truth; editing the file on disk diverges from it, skips validation (the cell never runs), and skips the browser broadcast. Make every cell change through siza (`insert_cell`/`replace_cell_source`/`delete_cell`) so it is applied, auto-run, and visible. Saving the session back to disk is the user's action in the browser, not a file you write.
16+
1517
## Discovery
1618

1719
```bash
@@ -59,6 +61,54 @@ ${CLAUDE_PLUGIN_ROOT}/skills/siza/scripts/siza-tool.sh insert_cell \
5961
# using the hash returned by insert_cell — never delete + re-insert.
6062
```
6163

64+
## Always write clear, notebook-friendly code
65+
66+
A notebook is a document a human reads top to bottom, not a script. Every cell you
67+
write or edit must be legible on its own. This is not optional polish; dense,
68+
mixed-concern cells make the notebook hostile to read.
69+
70+
- **Separate logic from display.** Pure computation (parsing, aggregation, models,
71+
ranking) goes in its own cells; rendering (tables, charts, `displayMarkdown`,
72+
widgets) goes in separate cells that call it. A cell that mixes a dense fold with the
73+
SVG that draws it is unreadable. This mirrors the library-vs-view split.
74+
- **One concept per cell.** Split a long cell that does several things into focused
75+
cells, each with a single responsibility and a one-line prose lead-in saying what
76+
question it answers. Never paste a whole subsystem into one cell.
77+
- **Readable, modular code.** Name things for what they are (`pathsBefore`, `growthFor`),
78+
not `rp'`/`mb'`/`gp`/`ks`. Extract helpers and a type alias or two; do not cram nested
79+
logic into one expression the reader must hold in their head. A short top-level comment
80+
states intent.
81+
- **Top-level bindings, not `do let`.** Write a cell's values as top-level definitions
82+
(`totalDelta = pTotal new - pTotal base`, `topRegressors = take 12 regressors`), not
83+
stuffed into a `do let … ` block. A `do let` forces every binding into one indented wall
84+
and pushes you to keep cramming; flat top-level bindings each stand alone and read in
85+
order. Reserve a trailing `do` for the actual effects (the `display*`/`showTable` calls).
86+
Top-level names are global (reactivity-tracked, can't collide across cells), so name them
87+
descriptively — never generic `top`/`lead`/`nm`.
88+
- **Shallow `where`/`let`.** Keep local bindings to one level of nesting and at most three
89+
per clause. The moment a `where` grows a nested `let` (a second level) or a fourth
90+
binding, lift that inner computation to its own top-level (or `-- compile`) helper,
91+
parameterised over what it needs. Deeply nested local scopes are the hardest notebook
92+
code to read.
93+
- **Narrate.** Put prose between cells that makes a claim the next cell substantiates.
94+
Hide setup (parsers, helpers) under a collapsed "Setup" heading; surface the results.
95+
- **Keep heavy work off the default path.** A cell that walks a large structure and is
96+
slow should not run on every load: gate it behind an on-demand helper (`drill "name"`)
97+
or compute only what the visible result needs.
98+
- **Compile heavy pure logic.** A tree walk or fold over a large structure in an
99+
*interpreted* cell can exceed the cell timeout and wedge the kernel. Put pure heavy
100+
functions in a `-- compile:` cell (native `-O2`) and call them from interpreted cells.
101+
A pure function can be compiled even when its arguments are interpreted runtime values:
102+
parameterise it over those values instead of closing over them (e.g. take `before`,
103+
`after` as `Prof` arguments rather than referencing the interpreted `base`/`new`).
104+
internal steps silently; it doesn't call them out as special sections. Drop headings like
105+
"Ruling out the false leads — renames" or "Excluding the pseudo-centres": the technique is
106+
built in, so describe what the reader sees in the result, not the implementation's
107+
housekeeping. Prose narrates findings, not the algorithm's defensive steps.
108+
- **Prose style.** Plain and detached, in the user's voice. No em dashes (use a colon or
109+
parens instead). No marketing or sales tone. No "X, not Y" / "rather than" contrasts.
110+
Headings are informative and gentle, not reveals ("What grew", not "The answer").
111+
62112
## Which mutation tool?
63113

64114
| You want to… | Use |
@@ -150,10 +200,12 @@ There is **one** GHCi kernel per notebook behind a single run-lock with no admis
150200

151201
- **NEVER run `execute_cell` (or any mutation) concurrently.** Issue one, wait for its response, then the next. In-flight requests queue **server-side**; the client `curl` gives up at 60 s but the **server keeps processing the backlog**, so the queue outlives every timeout.
152202
- **A timeout is not a failure and not a cancel.** When `execute_cell` returns a `curl` timeout, the cell is very likely still running (a cold `-O2` compile or a >60 s computation easily exceeds the curl ceiling). **STOP. DO NOT retry** — each retry adds another job to the queue and makes the wedge worse. Wait for the kernel to drain by polling the **lock-free** endpoints (`kernel_status`, `siza-tool.sh list_cells`, or `/api/ai/health`); they answer even while a cell holds the lock, which is how you tell "busy" from "wedged".
153-
- **Never attribute warm-up to the cell — the #1 way to measure wrong.** The *first* `execute_cell` after a (re)start, a `-- compile:` edit, or a `-- cabal:` edit blocks on the kernel cold-start **and** the `-O2` project compile (often 1–3 min) *before your cell even runs*, so its elapsed time is startup, not the cell. Do **not** benchmark or judge a cell from a cold call, and do not call a one-line cell "slow" because the first run took 130 s. **Warm the kernel first:** run a trivial cell (or poll `kernel_status` until `{kernel: alive, running: false}` with a *stable* `sessionGen`), *then* time the cell you care about. If you must drive cells programmatically, warm once and measure second — never measure the warm-up.
203+
- **Never attribute warm-up to the cell — the #1 way to measure wrong.** The *first* `execute_cell` after a (re)start, a `-- compile:` edit, or a `-- cabal:` edit blocks on the kernel cold-start **and** the `-O2` project compile (often 1–3 min) *before your cell even runs*, so its elapsed time is startup, not the cell. Do **not** benchmark or judge a cell from a cold call, and do not call a one-line cell "slow" because the first run took 130 s. **Warm the kernel first:** run a trivial cell (or poll `kernel_status` until `{kernel: alive, running: false, compiling: false}` with a *stable* `sessionGen``compiling: true` means it is still building, so `running: false` alone is not idle), *then* time the cell you care about. If you must drive cells programmatically, warm once and measure second — never measure the warm-up.
154204
- **Beware the Bash auto-background trap.** Long-running commands get auto-backgrounded, so an impatient retry of a slow loop (e.g. a per-cell read/sync loop) launches a *second concurrent* loop — several pile up and flood the queue. Killing the client (`pkill`/`Ctrl-C`) does **NOT** cancel the server-side work already queued. **Never retry a backgrounded siza loop** — let the first finish.
155205
- **Heavy exploration goes in the `scratchpad`,** never a long `execute_cell` against the live kernel — a blocking or long cell wedges the *whole* notebook for the user too.
156-
- **Recovery tools exist over the bridge:** `kernel_status` (lock-free — `{kernel: alive|absent, running, sessionGen}`), `interrupt`, `kernel_restart`, `export_notebook`. Use `kernel_status` to tell *warming/busy* from *wedged*; `interrupt` clears a runaway cell; `kernel_restart` gives a fresh kernel. If the kernel stays unresponsive **after** `kernel_restart`, only a **server-process restart by the user** recovers it (a fresh server also resets the cell timeout, configurable via `SABELA_CELL_TIMEOUT_SECONDS`).
206+
- **Recovery tools exist over the bridge:** `kernel_status` (lock-free — `{kernel: alive|absent, running, compiling, sessionGen}`), `interrupt`, `kernel_restart`, `export_notebook`. Use `kernel_status` to tell *warming/busy* from *wedged*; `interrupt` clears a runaway cell; `kernel_restart` gives a fresh kernel. If the kernel stays unresponsive **after** `kernel_restart`, only a **server-process restart by the user** recovers it (a fresh server also resets the cell timeout, configurable via `SABELA_CELL_TIMEOUT_SECONDS`).
207+
- **`running` and `compiling` are two separate axes — read both.** `running` is the run-lock (a cell or query executing); `compiling` is off-lock build work: a cabal-env install, a cold-start GHCi spawn, or a `-- compile:` module build. **`running: false` does NOT mean idle.** A cold start or a compile shows `{running: false, compiling: true}` for 1–3 min while a slow `execute_cell` appears to "hang" — that is the kernel *building*, not wedged: keep polling, do not retry or restart. Only `{running: false, compiling: false}` with a stable `sessionGen` is genuinely idle. A call that never returns while **both** are false (and `sessionGen` is unchanged) is the real wedge.
208+
- **Editing a `-- compile` cell wipes the whole interpreted context.** The recompile does a `:load` that clears *every* prompt binding (not just the edited module's dependents) — `base`, the analysis bindings, the chart helpers, all of it. After it, re-warm the interpreted chain **in notebook order** (params → load → analysis → helpers → displays), not just the cells the flags mark dirty: running them out of order gives transient `Variable not in scope` (a helper that references a not-yet-loaded binding), and the dependency tracker re-dirties downstream on each out-of-order run. A browser **Run All** is the clean re-warm. So batch `-- compile` edits, and verify a compiled-fn change by *timing it in a scratch cell* (the recompile wipe means a downstream `execute_cell` can also be measuring cold re-warm, not the function).
157209

158210
### After a `-- cabal:` change or a restart
159211

src/Sabela/AI/Capabilities/Kernel.hs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ module Sabela.AI.Capabilities.Kernel (
1616
import Control.Concurrent (forkIO)
1717
import Control.Monad (void)
1818
import Data.Aeson (Value, object, (.=))
19+
import Data.IORef (readIORef)
1920
import Data.Maybe (isJust)
2021
import Data.Text (Text)
2122

@@ -41,11 +42,13 @@ execKernelStatus app = do
4142
mSess <- getHaskellSession (appSessions app)
4243
busy <- maybe (pure False) ST.sbBusy mSess
4344
gen <- maybe (pure 0) ST.sbSessionGen mSess
45+
compiling <- readIORef (appBuilding app)
4446
pure $
4547
okOutcome $
4648
object
4749
[ "kernel" .= (if isJust mSess then ("alive" :: Text) else "absent")
4850
, "running" .= busy
51+
, "compiling" .= compiling
4952
, "sessionGen" .= gen
5053
]
5154

src/Sabela/Handlers.hs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import Data.Text (Text)
3939
import qualified Data.Text as T
4040
import qualified Data.Text.IO as TIO
4141

42+
import Data.IORef (readIORef)
4243
import qualified Sabela.AI.Store as AI
4344
import qualified Sabela.AI.Types as AI
4445
import qualified Sabela.Anthropic.Types as AI (cancel)
@@ -57,6 +58,7 @@ import Sabela.Handlers.Plan (
5758
executeFullRestart,
5859
executeRunAll,
5960
executeSingleCell,
61+
isSessionUpToDate,
6062
)
6163
import Sabela.Handlers.Shared
6264
import Sabela.Model (
@@ -67,7 +69,12 @@ import Sabela.Model (
6769
SessionStatus (..),
6870
cellLangOf,
6971
)
70-
import Sabela.Reactivity (cellStale, markDependentsDirty)
72+
import Sabela.Reactivity (
73+
cellStale,
74+
haskellCodeCells,
75+
markDependentsDirty,
76+
runAllNeedsRun,
77+
)
7178
import Sabela.State (App (..), getAIStore)
7279
import Sabela.State.NotebookStore (modifyNotebook, readNotebook)
7380
import ScriptHs.Parser (CabalMeta (..))
@@ -168,8 +175,17 @@ handleRunCell app cid = do
168175
handleRunAll :: App -> IO ()
169176
handleRunAll app = do
170177
debugLog app "[handler] handleRunAll"
171-
gen <- bumpGeneration app
172-
void $ forkIO $ executeRunAll app gen
178+
nb <- readNotebook (appNotebook app)
179+
building <- readIORef (appBuilding app)
180+
ready <- isSessionUpToDate app nb
181+
if not (runAllNeedsRun building ready (haskellCodeCells nb) nb)
182+
then
183+
debugLog
184+
app
185+
"[handler] handleRunAll: nothing to run (clean, or a build is in flight); skipping"
186+
else do
187+
gen <- bumpGeneration app
188+
void $ forkIO $ executeRunAll app gen
173189

174190
handleReset :: App -> IO ()
175191
handleReset app = do

src/Sabela/Handlers/Compile.hs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import Sabela.Model (
3838
OutputItem (..),
3939
)
4040
import qualified Sabela.SessionTypes as ST
41-
import Sabela.State (App (..), clearCompiledModules)
41+
import Sabela.State (App (..), clearCompiledModules, withBuilding)
4242
import Sabela.State.Environment (Environment (..))
4343
import Sabela.State.SessionManager (getHaskellSession)
4444
import qualified Sabela.Topo as Topo
@@ -93,7 +93,7 @@ compileChanged ::
9393
M.Map Text Text ->
9494
S.Set Text ->
9595
IO CompileOutcome
96-
compileChanged app gen backend cplan affectedCells changed orphans = do
96+
compileChanged app gen backend cplan affectedCells changed orphans = withBuilding app $ do
9797
let projDir = envTmpDir (appEnv app) </> "repl-project"
9898
forM_ (M.toList changed) $ \(name, src) -> do
9999
let path = projDir </> moduleFilePath name

src/Sabela/Handlers/Lifecycle.hs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ import Sabela.Session.Process (
5353
newSessionStreaming,
5454
)
5555
import qualified Sabela.SessionTypes as ST
56-
import Sabela.State (App (..), clearCompiledModules)
56+
import Sabela.State (App (..), clearCompiledModules, withBuilding)
5757
import Sabela.State.DependencyTracker (
5858
getHaskellDeps,
5959
getHaskellExts,
@@ -158,7 +158,7 @@ installAndRestart app gen metas = do
158158
else installDepsAndStartSession app gen metas
159159

160160
installDepsAndStartSession :: App -> Int -> CabalMeta -> IO Bool
161-
installDepsAndStartSession app _gen metas = do
161+
installDepsAndStartSession app _gen metas = withBuilding app $ do
162162
broadcastDepsStatus app metas
163163
setHaskellExts (appDeps app) (S.fromList (metaExts metas))
164164
let projDir = envTmpDir (appEnv app) </> "repl-project"

src/Sabela/Handlers/Plan.hs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ module Sabela.Handlers.Plan (
1313
executeSingleCell,
1414
executeFullRestart,
1515
executeRunAll,
16+
isSessionUpToDate,
1617

1718
-- * Sub-pieces (exposed for the entry-points module and tests)
1819
rerunBridgeCells,

src/Sabela/Reactivity.hs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
module Sabela.Reactivity (
44
ExecutionPlan (..),
55
cellStale,
6+
runAllNeedsRun,
67
computeExecutionPlan,
78
markDependentsDirty,
89
markAllInterpretedDirty,
@@ -60,6 +61,19 @@ computeFullExecutionPlan = computePlanCore Nothing
6061
cellStale :: Cell -> Bool
6162
cellStale c = cellDirty c || isJust (cellError c)
6263

64+
{- | Whether a run-all must actually execute. It is a no-op when a session
65+
build/restart is already in flight (so repeated run-all clicks can't stack
66+
restarts and back the kernel up) or when the session is ready and no cell is
67+
stale (the notebook is already current). A not-ready session always runs (it
68+
needs the cold-start). Keeps an accidental double-press from re-executing an
69+
unchanged notebook.
70+
-}
71+
runAllNeedsRun :: Bool -> Bool -> [Cell] -> Notebook -> Bool
72+
runAllNeedsRun building ready allCode nb
73+
| building = False
74+
| not ready = True
75+
| otherwise = not (null (epCellsToRun (computeStaleExecutionPlan allCode nb)))
76+
6377
{- | Plan for an incremental run-all: only stale cells and their
6478
transitive dependents run, in dependency order; clean cells keep their
6579
outputs. An all-clean notebook yields an empty plan.

src/Sabela/State.hs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ module Sabela.State (
44
App (..),
55
newApp,
66
clearCompiledModules,
7+
setBuilding,
8+
withBuilding,
79
getAIStore,
810
setAIStore,
911
configureAI,
@@ -30,6 +32,7 @@ import Control.Concurrent.MVar (
3032
newMVar,
3133
readMVar,
3234
)
35+
import Control.Exception (bracket_)
3336
import Data.Aeson (Value (..), eitherDecodeStrict, encode, object, (.=))
3437
import qualified Data.Aeson.Key as Key
3538
import qualified Data.Aeson.KeyMap as KM
@@ -89,12 +92,29 @@ data App = App
8992
{- ^ Per-session handle stores for external CLI clients, keyed by
9093
the @X-Sabela-Session@ header. Created lazily on first request.
9194
-}
95+
, appBuilding :: IORef Bool
96+
{- ^ True while the kernel is doing off-lock build work — installing a
97+
cabal env, spawning/cold-starting GHCi, or compiling a @-- compile@
98+
module. Distinct from the run-lock @running@ axis so a driver can tell a
99+
cold start from a hung cell ('kernel_status' surfaces it as @compiling@).
100+
-}
92101
}
93102

94103
-- | Forget which compiled modules the live session has loaded.
95104
clearCompiledModules :: App -> IO ()
96105
clearCompiledModules app = writeIORef (appCompiledModules app) M.empty
97106

107+
-- | Flip the off-lock build flag (see 'appBuilding').
108+
setBuilding :: App -> Bool -> IO ()
109+
setBuilding app = writeIORef (appBuilding app)
110+
111+
{- | Run an action with the build flag raised, lowering it again even on
112+
exception. Wrap cabal-env installs, cold starts, and @-- compile@ builds so
113+
'kernel_status' reports @compiling@ while they run.
114+
-}
115+
withBuilding :: App -> IO a -> IO a
116+
withBuilding app = bracket_ (setBuilding app True) (setBuilding app False)
117+
98118
-- | Read the current AI store (if configured).
99119
getAIStore :: App -> IO (Maybe AIStore)
100120
getAIStore = readMVar . appAI
@@ -228,6 +248,7 @@ newApp workDir globalDeps mHttpMgr mAiToken localPkgs = do
228248
<*> pure mHttpMgr
229249
<*> pure mAiToken
230250
<*> pure cliSessionsVar
251+
<*> newIORef False
231252

232253
-- | Resolve API key + saved model. Env ANTHROPIC_API_KEY wins for the key.
233254
resolveConfig :: FilePath -> IO (Maybe String, Maybe Text)

0 commit comments

Comments
 (0)