Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CLAUDE.md`:
- Around line 101-103: The documentation contradicts itself about available
memory backends: update the sentence that currently states "mem0 is the
currently only backend" to reflect the current reality described under the
memory/ and backends/ listings—either state that multiple backends are supported
(e.g., mem0, Qdrant+SQLite, inmemory, composite routing) or explicitly mark mem0
as the default/legacy backend; edit the CLAUDE.md section that mentions "mem0 is
the 'currently only backend'" to reference the backends/ subsection and align
wording (e.g., "mem0 (default/legacy) and other backends such as Qdrant+SQLite,
inmemory and composite routing are supported") so the memory/ and backends/
descriptions are consistent.

In `@src/synthorg/api/controllers/memory.py`:
- Around line 160-164: The preflight response omits the disk-space gate: ensure
you include the disk-space check result in the checks list before constructing
PreflightResult so can_proceed reflects storage limits. Call the existing
disk-space check helper (e.g. _run_disk_space_check or _disk_space_check) and
append its result to checks (the list produced by _run_preflight_checks()) prior
to building the tuple passed to PreflightResult (the same change must be applied
in both construction sites around the PreflightResult usage at lines shown).
Verify the aggregated can_proceed logic consumes that disk check result.
- Around line 226-244: Rollback currently only deactivates checkpoints and never
reapplies the saved backup config; after fetching cp with
SQLiteFineTuneCheckpointRepository.get_checkpoint and verifying
cp.backup_config_json you must deserialize that JSON and apply it to the live
deployment/config store (e.g., via a new or existing repository method such as
SQLiteFineTuneCheckpointRepository.apply_backup_config or
activate_checkpoint_with_config), ensure checkpoint active flags are updated
(instead of only calling deactivate_all) and persist the change, then re-query
with get_checkpoint and return the updated record; if no helper exists add a
repository method that accepts checkpoint_id and backup_config_json to perform
the config restore and activation.
- Around line 78-86: The handlers dereference app_state.fine_tune_orchestrator
unconditionally (e.g., the block that sets orchestrator =
app_state.fine_tune_orchestrator and calls await orchestrator.start(data));
guard this optional field before use and return the same controlled API error
used by get_fine_tune_status when fine-tuning is unavailable. Update each
affected handler (the start/trigger block around orchestrator.start, and the
similar uses at the other two locations referenced) to check if
app_state.fine_tune_orchestrator is None and if so return/raise the standardized
API error response; otherwise proceed to assign to orchestrator and call its
methods.
- Around line 160-161: The preflight probes run heavy synchronous work inside an
async handler: move both _run_preflight_checks() and _recommend_batch_size() off
the event loop by running them in threads and coordinating them with structured
concurrency; in the async controller handler, use asyncio.TaskGroup to launch
asyncio.to_thread(_run_preflight_checks, data) and
asyncio.to_thread(_recommend_batch_size) in parallel, await the TaskGroup,
collect the results into checks and batch_size, and propagate any exceptions so
the handler remains non-blocking.
- Around line 421-428: Update both _check_gpu and _recommend_batch_size to
access props.total_memory instead of props.total_mem when reading VRAM; in
addition, change _recommend_batch_size's exception handling to mirror
_check_gpu's pattern by re-raising system exceptions (MemoryError,
RecursionError) and only swallowing other exceptions (returning None) so system
errors propagate while other failures produce a None recommendation. Ensure you
modify the references to props and the except block around the computation in
_recommend_batch_size accordingly.

In `@src/synthorg/api/state.py`:
- Around line 389-395: set_fine_tune_orchestrator currently overwrites an
existing orchestrator silently; change it to first check
self._fine_tune_orchestrator and if it's already set log a WARNING/ERROR with
context and raise an exception to prevent silent reconfiguration, otherwise set
it and emit an INFO-level lifecycle transition log indicating the old->new
wiring; update the method (set_fine_tune_orchestrator) to follow the same
logging/guard pattern used by the other set_* lifecycle methods.

In `@src/synthorg/memory/backends/composite/adapter.py`:
- Around line 176-181: Add a brief clarifying comment above the nested async
function _check (and/or where backend: MemoryBackend = b is defined) explaining
that using a default parameter captures the current value of b to avoid Python's
late-binding closure issue; reference the async helper _check, the default arg
backend: MemoryBackend = b, results.append(await backend.health_check()), and
tg.create_task(_check()) so future maintainers understand this pattern and why
the default parameter is required.

In `@src/synthorg/memory/backends/inmemory/adapter.py`:
- Around line 336-346: The clear() method is synchronous while other CRUD
methods (store, retrieve, get, delete, count) are async; change clear to an
async method (async def clear(self, agent_id: NotBlankStr) -> int) and call
await self._require_connected() at the start for consistency with other
operations, then pop the agent store (self._store.pop(str(agent_id), {})) and
return its length as before so behavior is unchanged but the API and connection
checks match the other methods.
- Around line 81-87: The disconnect method lacks the same _connect_lock
protection used by connect, creating a race between connect() and disconnect();
wrap the body of disconnect (the checks and setting of self._connected and
logger calls—MEMORY_BACKEND_DISCONNECTING and MEMORY_BACKEND_DISCONNECTED) in
the same async lock acquisition used by connect (i.e., acquire the instance's
_connect_lock via async context or explicit acquire/release), perform the
idempotent check and state change while holding the lock, and then release the
lock so connect() and disconnect() are symmetric and thread-safe.

In `@src/synthorg/memory/embedding/cancellation.py`:
- Around line 7-45: The module lacks structured logging: import get_logger from
synthorg.observability and create logger = get_logger(__name__), then add
logger.info in CancellationToken.cancel to record the state transition (include
context like token id or class name), add logger.debug or info in the
is_cancelled property access if desired, and add logger.warning or logger.error
in CancellationToken.check immediately before raising FineTuneCancelledError
(include the same context and the message "Fine-tuning pipeline run was
cancelled"); update references to CancellationToken.cancel, .is_cancelled, and
.check to ensure logs are emitted where state changes or errors occur.

In `@src/synthorg/memory/embedding/fine_tune_models.py`:
- Around line 245-294: The FineTuneRun model allows inconsistent terminal-state
combinations (e.g., stage == FineTuneStage.FAILED with error None, terminal
stages with completed_at None, or non-terminal stages having completed_at set);
add a model_validator(mode="after") on FineTuneRun that enforces these
invariants by checking the stage, error, completed_at, and stages_completed
fields and raising ValueError with clear messages when invariants are violated
(examples: when stage.is_terminal() then ensure completed_at is not None; when
stage == FineTuneStage.FAILED ensure error is not None; when not
stage.is_terminal() ensure completed_at is None; validate that stages_completed
only contains stages prior to current stage and that progress is None for
terminal stages or within 0..1 otherwise). Ensure the validator runs after
initialization and references FineTuneRun, stage, error, completed_at,
stages_completed, and progress to locate the code to change.

In `@src/synthorg/memory/embedding/fine_tune_orchestrator.py`:
- Around line 197-201: The cancel() method currently signals cancellation via
self._cancellation.cancel() but returns before the background task finishes;
update cancel() to await the task stored in self._cancellation (e.g., await
self._cancellation) so callers know the run has stopped, and handle/ignore
asyncio.CancelledError and optional timeout using asyncio.wait_for if needed,
then log MEMORY_FINE_TUNE_CANCELLED after the awaited task completes; reference
the existing cancel(), self._cancellation, and MEMORY_FINE_TUNE_CANCELLED
symbols when making this change.
- Around line 457-483: The progress callback _cb mutates shared state
self._current_run from threads; either document the thread-safety assumption or
make updates thread-safe by scheduling the mutation on the event loop or using a
lock: inside _cb (returned by _make_progress_cb) avoid direct assignment from
worker threads and instead obtain the main asyncio loop and call
loop.call_soon_threadsafe(...) to perform the model_copy + self._current_run
assignment and _emit_ws, or protect the assignment with a threading.Lock; update
the code around references to self._current_run, run_id, run, and _emit_ws
accordingly and add a short comment describing the chosen approach.

In `@src/synthorg/memory/embedding/fine_tune.py`:
- Around line 224-226: The extractive-query fallback in the function that builds
the query uses sentences = chunk.split(".") and first = sentences[0].strip()
which can yield an empty string or an overly long query; update the logic to
trim chunk first, check if the trimmed chunk is empty and return a safe default
(e.g., "Find information about: [no content]") or a short placeholder, and if
not empty use the first sentence but cap its length (e.g., first_sentence[:100])
so the returned string from the function (the "Find information about: {first}"
return) is neither empty nor unbounded; reference variables sentences, first and
chunk when applying these checks.

In `@src/synthorg/persistence/sqlite/schema.sql`:
- Around line 418-426: The fine_tune_checkpoints table allows arbitrary integers
and multiple rows marked active; change the column definition for is_active to
enforce boolean values (INTEGER NOT NULL DEFAULT 0 CHECK(is_active IN (0,1)))
and add a schema-level uniqueness constraint for the single-active invariant by
creating a partial unique index that enforces at most one row with is_active=1
(e.g., CREATE UNIQUE INDEX ... ON fine_tune_checkpoints(is_active) WHERE
is_active=1); update the existing idx_ftc_active to the new CHECK and add the
partial unique index (referencing the table fine_tune_checkpoints, column
is_active and index name idx_ftc_active / new unique index name) so the DB
prevents invalid values and multiple active rows.

In `@tests/unit/memory/backends/composite/test_adapter.py`:
- Around line 293-384: The suite lacks a degraded fan-out test: add a new async
test method (e.g., test_retrieve_degraded_fanout) in TestRetrieveFanout that
constructs a CompositeBackend with one child backend whose retrieve() raises and
one healthy child that returns entries, then call connected.retrieve (or
CompositeBackend.retrieve) with an appropriate MemoryQuery and assert the
composite returns the healthy child's entries (ignoring the failing child) and
preserves merge ordering across children; implement the failing child as a small
stub/mock backend whose retrieve raises an exception, register both children
into the CompositeBackend used by the test, and assert results contain only the
healthy contents and are ordered as expected.

In `@tests/unit/memory/backends/inmemory/test_adapter.py`:
- Around line 229-245: Add tests that verify TTL/expiration behavior by mocking
time.monotonic() and asyncio.sleep() around existing tests like
test_get_missing_returns_none and test_namespace_preserved: store an entry with
a short TTL using connected.store(..., ttl=...), assert it is returned by
connected.get(), connected.retrieve(), and counted by connected.count(), then
advance the mocked monotonic time past the TTL (or await a mocked asyncio.sleep)
and assert the entry is no longer returned by connected.get(),
connected.retrieve(), and that connected.count() decrements; reference the
InMemoryBackend instance named connected and its methods get, store, retrieve,
and count when adding these assertions.

In `@tests/unit/memory/embedding/test_fine_tune_orchestrator.py`:
- Around line 64-74: The helper _request currently creates real files under
tmp_path (src/doc.txt) but returns a FineTuneRequest with synthetic POSIX paths
(/test/src, /test/out), which is confusing; update _request to either stop
creating the filesystem artifacts if they are unused, or create the files at the
same paths used in the FineTuneRequest (or make the intent explicit) — modify
the function _request so it either (a) removes the tmp_path/src creation and
doc.txt write, or (b) set source_dir and output_dir to the real tmp_path-based
paths (e.g., str(src) and str(tmp_path/"out")), or add a clear docstring above
_request explaining that mocks bypass file access and the created files are
intentionally unused; pick one consistent approach and update tests that rely on
_request accordingly.
- Around line 101-102: The test uses fixed asyncio.sleep() calls to wait for
background work (in tests/unit/memory/embedding/test_fine_tune_orchestrator.py);
replace these timing-based waits by awaiting a deterministic completion signal
from the orchestrator: modify the orchestrator to expose a completion
Future/Event or its running Task (e.g., orchestrator._current_task or
orchestrator.completion_event) and change the test to await that signal (e.g.,
await orchestrator._current_task or await orchestrator.completion_event.wait())
so tests block deterministically instead of using asyncio.sleep(0.05/0.1/0.2);
alternatively, mock the async timing functions (asyncio.sleep/time.monotonic) in
the test if exposing a completion signal is not feasible.
- Around line 221-224: The test is coupling to CancellationToken internals by
calling token._event.wait; add a public blocking API on CancellationToken (e.g.,
wait_until_cancelled or wait_until_cancelled_async) and update the test to call
that instead of token._event.wait; implement the new method to delegate to the
internal threading.Event.wait (or await via asyncio.to_thread) and preserve
existing behavior (return/raise consistently with token.check()), and keep the
token.check() call in the test unchanged so the test still verifies cancellation
behavior using the public method.

In `@tests/unit/memory/test_models.py`:
- Around line 118-133: Combine the two duplicate tests into a single
parameterized test: replace test_namespace_empty_rejected and
test_namespace_whitespace_rejected with a `@pytest.mark.parametrize` decorated
function (e.g. test_invalid_namespace_rejected) that iterates over tuples of
(namespace, expected_match) and asserts
MemoryStoreRequest(category=MemoryCategory.WORKING, namespace=namespace,
content="c") raises ValidationError with the expected_match. Reference
MemoryStoreRequest and MemoryCategory.WORKING in the test and keep the same
ValidationError assertions for the provided namespaces "" and "   ".

In `@web/src/pages/fine-tuning/CheckpointTable.tsx`:
- Around line 39-114: The checkpoint row JSX inside the checkpoints.map is too
large and should be extracted into a new presentational component, e.g.,
CheckpointTableRow; create a new component that accepts a checkpoint prop plus
the actions/handlers (deployCheckpointAction, rollbackCheckpointAction,
setDeletingId) and any helpers used (formatBytes, MetricDelta, StatusBadge,
Button) and move the entire <tr> rendering logic into it, then replace the map
body with checkpoints.map(cp => <CheckpointTableRow key={cp.id} checkpoint={cp}
deploy={deployCheckpointAction} rollback={rollbackCheckpointAction}
setDeletingId={setDeletingId} />) so all formatting, status display and mutation
triggers remain unchanged but live in the new component.

In `@web/src/pages/fine-tuning/DependencyMissingBanner.tsx`:
- Around line 3-14: Replace the hand-rolled div in the DependencyMissingBanner
component with the shared UI primitives from web/src/components/ui/—at minimum
use SectionCard to wrap the content, and compose it with the appropriate
header/content subcomponents (e.g., CardHeader/CardTitle or the SectionCard
title prop and CardContent) and the existing code/mono text primitive from the
ui library instead of raw <code> markup; preserve the message text and
role="alert" semantics and ensure classes for spacing/typography are removed in
favor of the ui components so the banner follows the dashboard design system.

In `@web/src/pages/fine-tuning/PipelineControlPanel.tsx`:
- Around line 67-72: The advanced controls (rendered when showAdvanced is true)
are no-ops because InputField instances for "Epochs", "Learning Rate", and
"Batch Size" use fixed values and empty onChange handlers and the start action
(StartFineTuneRequest) only sends source_dir; fix by adding component state
(e.g., epochs, learningRate, batchSize initialized from
preflight?.recommended_batch_size) and pass them as value props, implement
onChange handlers to update those state variables, and include these fields in
the payload built by StartFineTuneRequest (or alternatively hide the entire
showAdvanced block until these fields are fully supported).
- Around line 14-26: Preflight validation is being reused across edits to
sourceDir so stale results can gate new requests; update the code to invalidate
the preflight state whenever the request changes by clearing the preflight
before running a new check or when sourceDir is modified: call the state setter
that clears the preflight result (e.g., setPreflight(undefined) or equivalent)
inside setSourceDir's updater and immediately before calling runPreflightCheck
in handlePreflight and before calling startRun in handleStart (and similarly in
the other handlers referenced on lines ~34-35 and ~49) so each new request
triggers a fresh validation.

In `@web/src/pages/fine-tuning/PipelineProgressBar.tsx`:
- Around line 9-10: The computed percentage pct in PipelineProgressBar uses
progress directly, which can produce values outside 0–100; clamp progress to the
valid range before converting (e.g., ensure progress is between 0 and 1, then
compute pct) so that aria-valuenow and the width style always receive values
within 0–100; update the logic around the pct calculation and any other usages
(e.g., the aria-valuenow attribute and the inline width style) to use the
clamped value derived from the progress prop or state.
- Around line 19-31: The PipelineProgressBar component is re-implementing a
progress primitive using raw divs (pct variable); replace that markup with the
shared UI Progress component from web/src/components/ui (import Progress and its
child ProgressIndicator or the appropriate API), pass pct as the
value/aria-valuenow equivalent and keep aria-label, min/max props, and the same
classes/transition via existing component props or wrapper className so styling
and behavior match the app-wide progress primitive.

In `@web/src/pages/fine-tuning/PipelineStepper.tsx`:
- Around line 24-59: The connector uses the current step's isComplete so the bar
into the active step stays muted; fix by computing connectorComplete = i <=
currentIdx (while keeping isComplete = i < currentIdx and isCurrent = s.key ===
stage) and use connectorComplete for the connector class; also extract the JSX
inside STAGES.map into a new StepItem (or StepRenderer) component that accepts
props (s, i, currentIdx, stage) and renders the connector, circle and label,
then replace the inline map body with <StepItem ... /> to keep the map simple.

In `@web/src/pages/fine-tuning/RunHistoryTable.tsx`:
- Around line 7-11: STAGE_STATUS_MAP in RunHistoryTable.tsx currently maps only
complete/failed/idle so in-progress stages fall back to 'idle'; update
STAGE_STATUS_MAP to include the in-progress stage keys generating_data,
mining_negatives, training, evaluating, and deploying and map them to 'active'
(so the component that uses STAGE_STATUS_MAP to render badges shows a non-idle
badge for in-progress stages).
- Around line 38-63: Extract the JSX inside runs.map into a new presentational
component (e.g., RunHistoryRow) that accepts a single prop "run" and reuses
existing helpers (formatDuration, formatStage), constants (STAGE_STATUS_MAP),
and components (StatusBadge); replace the inline JSX in RunHistoryTable's
runs.map with <RunHistoryRow key={run.id} run={run} /> and implement
RunHistoryRow to render the same <tr> structure, preserving the key, date
formatting (new Date(run.started_at).toLocaleString()), duration fallback,
stages_completed count, and run.config.source_dir so the table behavior remains
identical but the map body is simplified.
- Around line 70-75: The formatDuration function incorrectly uses Math.round for
minute calculations which can roll over to 60m; update formatDuration so seconds
(<60) still use Math.round, but for minute-only and hour+minute branches compute
minutes with Math.floor((seconds % 3600) / 60) or Math.floor(seconds / 60)
respectively (and compute hours with Math.floor(seconds / 3600)), ensuring no
60-minute value appears and no extra rollover logic is needed.

In `@web/src/stores/fine-tuning.ts`:
- Around line 107-114: runPreflightCheck never clears prior errors, so add
clearing of the error state when starting and on successful completion: in
runPreflightCheck (the async function that calls runPreflight) update the
initial set call to include error: null (e.g., set({ loading: true, preflight:
null, error: null })) and ensure the success branch also clears error (e.g.,
set({ preflight: result, loading: false, error: null })); leave the catch to set
the failure message as it does.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/design/page-structure.md`:
- Around line 207-208: The Fine-Tuning sub-page documentation for the memory
namespace (/settings/memory/fine-tuning) omits the checkpoint lifecycle; update
the page-structure text for the Fine-Tuning sub-page to include a concise IA
spec and route note that document checkpoint list, deploy, rollback, and delete
flows (including UI affordances, required API actions, user permissions, and
expected status transitions) alongside the existing pipeline status/history,
preflight, and start/cancel descriptions so operators can discover and manage
checkpoints from the same route.

In `@src/synthorg/api/controllers/memory.py`:
- Around line 248-258: Wrap the activation + settings update in a try/except so
activation is not left half-done: after calling repo.set_active(checkpoint_id)
attempt the settings updates via app_state.settings_service.set(...) inside a
try block; on any exception catch it, roll back the repo activation by restoring
the previous active checkpoint (use repo.get_checkpoint to read prior active id
before changing, or call a repo method to unset/deactivate the new checkpoint)
and re-raise or return a controlled error, and stop relying on
cp.backup_config_json to gate settings updates (use the incoming checkpoint's
model_path or an explicit flag instead) so settings updates reflect the current
deployment intent. Ensure you reference repo.set_active, repo.get_checkpoint,
and app_state.settings_service.set in the fix.
- Around line 284-291: The code assumes cp.backup_config_json is valid JSON and
that repo.get_checkpoint(checkpoint_id) returns a value; update the handler to
catch json.JSONDecodeError around json.loads(cp.backup_config_json) (log the
error with context and skip applying the corrupted backup or raise a controlled
ApiError) and add a null check after updated = await
repo.get_checkpoint(checkpoint_id) to handle a None result (return a 404
ApiError or appropriate ApiResponse error instead of returning None). Use the
existing symbols svc.set, repo.deactivate_all, repo.get_checkpoint,
cp.backup_config_json and ApiResponse/ApiError to locate and implement the fix.

In `@src/synthorg/memory/backends/composite/adapter.py`:
- Around line 382-423: The _fanout_retrieve path currently converts all child
exceptions into warnings and returns an empty results list even when every
backend failed; modify the logic in the async TaskGroup/_fetch closure to track
successful backend completions (e.g., increment a success_count or append
successful backend IDs when entries are retrieved) and after the TaskGroup
finishes, if success_count == 0 raise MemoryRetrievalError (include aggregated
errors for context) instead of returning [] ; keep existing logging for partial
failures (errors list and MEMORY_COMPOSITE_FANOUT_PARTIAL) but ensure callers
see an exception when no backends succeeded.

In `@src/synthorg/memory/backends/inmemory/adapter.py`:
- Around line 171-177: store() currently counts agent_store length before
removing expired entries so expired memories still consume quota; update store()
to prune expired items from self._store[str(agent_id)] before checking len and
enforcing self._max_memories_per_agent, reusing the same expiry logic used in
retrieve() (or extract that logic into a helper like _prune_expired(agent_id)
and call it from store(), retrieve(), and any direct lookup/count methods) so
counts and lookups consistently ignore expired entries and new writes are
allowed once expired items are pruned.

In `@src/synthorg/memory/embedding/fine_tune.py`:
- Around line 155-157: Validate the validation_split input at the start of the
fine-tune routine (the function that accepts validation_split, ProgressCallback,
CancellationToken) and before any dataset slicing: reject values not strictly
between 0 and 1 and compute split indices such that both train and validation
get at least one example (e.g., enforce min_validation = 1 and min_train = 1,
adjust split to floor/max accordingly), throwing a clear
ValueError/ArgumentError if the dataset is too small or the split cannot satisfy
these constraints; apply the same checks where validation_split is used later
(the alternative slicing path referenced around the current use) so you never
attempt to slice into empty train/validation sets.
- Around line 628-641: The function deploy_checkpoint currently logs config_path
but never applies it; ensure that when settings_service is None and a
config_path is provided you perform the file-based update (backup existing file,
modify the embedder config entry to point to checkpoint_path, and atomically
write the updated config), and when neither settings_service nor config_path can
be used, raise/return an error before logging success; locate deploy_checkpoint
and the references to config_path and settings_service and either implement the
backup+modify+write flow for the config file or explicitly reject that code path
so deployment cannot falsely report success.

In `@src/synthorg/persistence/sqlite/fine_tune_repo.py`:
- Around line 136-143: Change parenthesized except clauses to PEP 758
comma-separated form: replace constructs like "except (sqlite3.Error,
aiosqlite.Error) as exc:" with "except sqlite3.Error, aiosqlite.Error as exc:"
(and similarly for other multi-exception handlers). Update the occurrence in the
fine-tune save block that logs MEMORY_FINE_TUNE_PERSIST_FAILED and raises
QueryError (the except handling around run.id and logger.exception), and apply
the same pattern for all other multi-exception excepts in this module (the other
occurrences noted in the review) so they use the comma-separated syntax without
parentheses.

In `@web/src/pages/fine-tuning/PipelineProgressBar.tsx`:
- Around line 8-30: PipelineProgressBar currently coerces a null progress to 0%
which misrepresents an indeterminate state; update PipelineProgressBar (props:
stage, progress) to treat progress === null as indeterminate by: 1) not
computing/printing pct or the "{pct}%" label when progress is null, 2) omitting
aria-valuenow from the outer progressbar element in that case (keep
aria-valuemin/max), and 3) rendering an indeterminate visual (e.g., a full-width
animated/striped/gradient inner bar or a CSS class like "indeterminate") instead
of setting style.width; make the conditional logic around pct, aria-valuenow and
the inner bar appearance in the PipelineProgressBar function so assistive tech
and visuals reflect indeterminate states correctly.

In `@web/src/pages/FineTuningPage.tsx`:
- Around line 28-31: The component currently only registers a local handler via
useWebSocketStore's onChannelEvent/offChannelEvent, but doesn't request server
delivery for the 'system' channel so progress can stall; update the effect that
registers the handler (where useWebSocketStore, useShallow, onChannelEvent and
offChannelEvent are used) to also call the store's channel subscription API to
subscribe to 'system' when mounting and unsubscribe from 'system' in the cleanup
— i.e., grab the subscribe/unsubscribe functions from useWebSocketStore, call
subscribe('system') before registering the handler, and call offChannelEvent
followed by unsubscribe('system') in the effect cleanup.

---

Duplicate comments:
In `@src/synthorg/api/controllers/memory.py`:
- Around line 388-396: The preflight flow in _run_preflight_checks currently
omits disk-space validation; add a call to the disk-space check (e.g.
_check_disk_space(...)) into the checks list so disk usage is validated along
with _check_dependencies, _check_gpu, and _check_documents; update
_run_preflight_checks to append _check_disk_space(request.source_dir) (or
appropriate path argument) so the returned checks include the disk-space
PreflightCheck used by can_proceed.

In `@src/synthorg/memory/embedding/fine_tune_orchestrator.py`:
- Around line 213-222: The cancel() method currently sets self._cancellation and
logs MEMORY_FINE_TUNE_CANCELLED but does not wait for the background pipeline to
finish; update cancel() (protected by self._op_lock) to await the background
task's completion (e.g., wait on the task/future that is canceled or a stored
asyncio.Task representing the run) with a reasonable timeout, handle
asyncio.TimeoutError and cancellation exceptions by logging and returning, and
ensure the method returns only after the run task has terminated (or document in
the docstring if you intentionally want fire-and-forget behavior). Reference
symbols: cancel(), self._op_lock, self._cancellation, the background task/future
variable that represents the active run, logger, and MEMORY_FINE_TUNE_CANCELLED.

In `@tests/unit/memory/embedding/test_fine_tune_orchestrator.py`:
- Around line 64-74: The test helper _request currently creates real files under
tmp_path but returns a FineTuneRequest with synthetic POSIX paths ("/test/src",
"/test/out") which can be confusing; update the _request function to include a
concise docstring that explains it deliberately writes local artifacts for test
diagnostics while returning synthetic POSIX paths because the FineTuneRequest is
used with an orchestrator mock that does not access the filesystem, and mention
FineTuneRequest and the tmp_path behavior so future maintainers understand the
intentional mismatch.

In `@web/src/pages/fine-tuning/CheckpointTable.tsx`:
- Around line 45-120: The map callback in CheckpointTable.tsx contains a large
chunk of JSX for each row — extract that JSX into a new presentational component
(e.g., CheckpointRow) that accepts a checkpoint object (cp) and the action
handlers (deployCheckpointAction, rollbackCheckpointAction, setDeletingId) as
props; move all logic for date/bytes/metric formatting, StatusBadge/MetricDelta
usage, and the conditional Buttons into that component, then replace the inline
map body with <CheckpointRow cp={cp} ... /> to keep the .map() concise and
reuseable while preserving existing symbols like MetricDelta, StatusBadge,
formatBytes, and the three action functions.

In `@web/src/pages/fine-tuning/PipelineControlPanel.tsx`:
- Around line 24-33: The advanced fields (epochs, learningRate, batchSize) are
being sent as hard defaults even when Advanced is collapsed and batchSize never
picks up a later preflight value; change their handling so they act as optional
overrides: stop initializing batchSize/epochs/learningRate to concrete positive
defaults (e.g. '3'/'1e-5'/'128'); instead initialize them to undefined or empty
string and when a preflight arrives set batchSize only if the user hasn't
entered an override (useEffect watching preflight to call
setBatchSize(preflight.recommended_batch_size) only when current batchSize is
empty). Also modify buildRequest() to include epochs, learning_rate, and
batch_size only if the user has explicitly set them or Advanced is open (check
the Advanced toggle state) rather than always sending the fields. Reference:
epochs, setEpochs, learningRate, setLearningRate, batchSize, setBatchSize,
useFineTuningStore.preflight, buildRequest.

In `@web/src/pages/fine-tuning/PipelineStepper.tsx`:
- Around line 24-60: Extract the JSX inside STAGES.map in PipelineStepper into a
new presentational component (e.g., StepItem) to keep the map callback simple;
move the logic that computes isComplete, isCurrent, connectorComplete and the
rendering of the connector div, badge div (checkmark/number), and label span
into that component, accept props like stage, index, currentIdx, and key, and
replace the inline block in STAGES.map with a single StepItem call so the map
callback is under 8 lines and the state handling stays the same.

In `@web/src/pages/fine-tuning/RunHistoryTable.tsx`:
- Around line 45-70: Extract the JSX inside runs.map into a new presentational
component (e.g., RunRow) that accepts a single prop like run: RunSummary (or the
existing run type), moves all formatting/logic (new
Date(run.started_at).toLocaleString(), formatDuration, STAGE_STATUS_MAP lookup,
formatStage, run.config.source_dir, run.stages_completed.length) into that
component, and returns the <tr> markup currently inline; update runs.map to just
render <RunRow key={run.id} run={run} /> and ensure RunRow is typed/exported
appropriately and imports StatusBadge, formatDuration, formatStage, and
STAGE_STATUS_MAP so the visual output and classNames remain identical.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/synthorg/api/controllers/memory.py`:
- Around line 425-428: The preflight is calling _check_disk_space with
request.source_dir but the pipeline writes to request.output_dir; change the
call in the checks list to use request.output_dir (i.e., replace
_check_disk_space(request.source_dir) with
_check_disk_space(request.output_dir)) so disk-space is validated against the
actual write destination (or optionally call _check_disk_space for both
request.source_dir and request.output_dir if you want to validate both mounts).

In `@src/synthorg/memory/backends/composite/adapter.py`:
- Around line 357-368: The method _resolve_retrieve_targets currently treats
only query.namespaces is None as "no filter" and returns all backends, while an
empty frozenset returns no targets; update the logic in
_resolve_retrieve_targets to treat falsy namespaces (e.g., empty frozenset) the
same as None by checking "if not query.namespaces" so empty sets return
list(self._unique_backends), leaving the rest of the resolve loop (seen dict +
self._resolve(ns)) unchanged; reference _resolve_retrieve_targets,
MemoryQuery.namespaces, self._unique_backends, and self._resolve when making
this change.
- Around line 370-426: The except clause inside _fanout_retrieve currently uses
"except MemoryError, RecursionError:" which binds to builtins.MemoryError and is
syntactically wrong for grouping; update the imports to include the synthorg
custom MemoryError from synthorg.memory.errors and change the clause to properly
group exceptions (e.g., except (MemoryError, RecursionError):) so synthorg's
MemoryConnectionError/MemoryRetrievalError are re-raised instead of being
swallowed; reference the import block that imports from synthorg.memory.errors
and the _fanout_retrieve function's except clause to make the fixes.

In `@src/synthorg/memory/backends/inmemory/adapter.py`:
- Around line 244-272: The get() method currently returns entries even if
expired, causing inconsistency with retrieve(); update get(self, agent_id,
memory_id) to check the entry with the existing _is_expired(entry) helper and
return None when expired (and optionally log an expiration event similar to
MEMORY_ENTRY_FETCHED); ensure you reference the same keys used in _store lookup
and keep the existing logger usage (MEMORY_ENTRY_FETCHED) but add a debug or
trace log for expired entries so semantics match retrieve().
- Around line 304-335: count() currently tallies all entries from self._store
regardless of expiry, causing mismatch with retrieve() which filters out expired
items; update count() (in src/synthorg/memory/backends/inmemory/adapter.py) to
apply the same expiration check used by retrieve() when iterating
agent_store.values() (i.e., skip entries where the entry is expired based on its
expires_at/expiration logic or by calling the same helper used by retrieve()),
preserve the category filter logic, and log MEMORY_ENTRY_COUNTED with the
filtered total so counts and retrievals are consistent.

In `@src/synthorg/memory/embedding/fine_tune.py`:
- Around line 706-712: The call to SettingsService.set is passing the timestamp
as a positional 4th argument which violates the keyword-only parameter
expected_updated_at; update the call in fine_tune.py to call
settings_service.set("memory", "embedder_model", checkpoint_path,
expected_updated_at=now) so expected_updated_at is passed by name (refer to
settings_service.set and the local variables checkpoint_path and now) and remove
the positional argument usage.

In `@tests/unit/memory/embedding/test_fine_tune_orchestrator.py`:
- Around line 128-130: The test currently suppresses all exceptions from
awaiting orchestrator._current_task using contextlib.suppress(Exception); change
this to either explicitly assert that the background task was cancelled (await
and expect asyncio.CancelledError) or catch only the concrete cancellation
exception type instead of Exception, e.g., handle asyncio.CancelledError around
awaiting orchestrator._current_task (and similarly for the second occurrence at
lines 150-152) so unrelated errors are not silently swallowed and the test fails
for unexpected failures.

---

Duplicate comments:
In `@src/synthorg/api/controllers/memory.py`:
- Around line 252-262: The activation flow updates settings and checkpoint state
in separate commits which can leave them inconsistent on partial failure; modify
the logic around app_state.settings_service (svc) and repo operations
(repo.set_active, repo.deactivate_all) so the cross-store transition is atomic:
either (A) add a new service method on settings_service that performs both the
settings writes (e.g., embedder_model/embedder_provider) and the checkpoint
activation in one operation, or (B) capture the prior settings values before
calling svc.set (read current "memory.embedder_model" and
"memory.embedder_provider"), perform the two svc.set calls, then call
repo.set_active/prior handling, and on any exception restore the captured prior
settings values and the prior checkpoint state (using repo.set_active(prior.id)
or repo.deactivate_all()); ensure the rollback path updates both the settings
and repo consistently and reference cp.model_path, svc.set, prior,
repo.set_active, and repo.deactivate_all in your changes.

In `@tests/unit/memory/embedding/test_fine_tune_orchestrator.py`:
- Around line 64-81: The test helper _request() currently returns synthetic
"/test/..." paths which makes tests write to the filesystem root and leaves
source_dir out of sync with the files you created; change it to point at the
real tmp_path locations you create (e.g. use the src Path you create and an
output dir under tmp_path) and return them as POSIX strings so FineTuneRequest
still rejects Windows drive letters; ensure you also create the output directory
before returning so _mock_all_stages() can write files there (references:
_request, FineTuneRequest, _mock_all_stages).

In `@web/src/pages/fine-tuning/PipelineProgressBar.tsx`:
- Around line 23-38: Replace the hand-rolled progress markup inside the
PipelineProgressBar component with the shared UI progress primitive: import the
shared Progress component and use it instead of the raw divs, passing pct as the
determinate value (e.g., value={pct}) and indeterminate as the prop to toggle
the indeterminate state, and preserve aria-label ("Fine-tuning progress") and
aria min/max when determinate; remove the duplicated determinate/indeterminate
div branches and any inline style-based width logic so the shared component
handles styling, animation and accessibility.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/synthorg/memory/embedding/fine_tune.py`:
- Around line 3-5: The module-level docstring incorrectly claims "LLM-powered"
synthetic data generation while the implementation of _generate_query ignores
llm_provider and always uses an extractive fallback; update the module docstring
to state that synthetic data generation currently uses an extractive fallback
(no LLM calls) and reference the limitation of _generate_query, or alternatively
implement LLM usage inside _generate_query to match the docstring—ensure the
module docstring and _generate_query behavior (and its docstring) are
consistent.
- Around line 720-745: The backup_config.json is written unconditionally via
backup_path.write_text even when deployment may be skipped; move the backup
write so it only happens after you confirm deployment will proceed (i.e., after
checking settings_service or config_path and before calling settings_service.set
or writing the config), or alternatively only write when returning a json backup
for downstream use; update the logic around backup_path/backup and the
conditional using settings_service, config_path, checkpoint_path and
MEMORY_FINE_TUNE_CHECKPOINT_DEPLOYED so orphaned backup files are not created
when the function logs the warning and returns early.
- Around line 708-718: The current broad except block around the
settings_service.get call swallows all Exceptions; change it to only catch
settings-related errors by replacing the generic "except Exception" with "except
SettingsError" (while still re-raising MemoryError and RecursionError) so
SettingNotFoundError, SettingsEncryptionError, QueryError, etc. are logged via
MEMORY_FINE_TUNE_BACKUP_READ_SKIPPED and other unexpected exceptions can
propagate; locate the try/except surrounding settings_service.get(...) in the
fine_tune backup logic (references: settings_service.get, backup,
MEMORY_FINE_TUNE_BACKUP_READ_SKIPPED) and update the except clause accordingly.

In `@src/synthorg/persistence/sqlite/fine_tune_repo.py`:
- Around line 40-55: Map row-parsing failures to the repository's QueryError:
wrap calls that can raise (datetime.fromisoformat in _parse_ts and
_parse_required_ts, any json.loads and Pydantic model_validate_json usages that
map DB rows) in a try/except that catches ValueError, TypeError, and
pydantic.ValidationError and re-raises as QueryError with context (similar to
the wrapper used in _checkpoint_from_row); apply the same pattern for the
row-to-model mapping code paths used by list_runs() and list_checkpoints() so a
single corrupted row produces a QueryError instead of bubbling raw exceptions.
- Around line 129-131: The code persists
run.started_at/created_at/updated_at/completed_at by calling .isoformat() which
preserves timezone offsets and breaks lexical ordering of TEXT columns; update
the persistence logic in fine_tune_repo.py to normalize datetimes to UTC before
formatting (e.g., call .astimezone(datetime.timezone.utc) on run.started_at,
run.created_at, run.updated_at and run.completed_at when building the
VALUES/parameters) and then store the UTC ISO string (optionally with 'Z' for
+00:00) so ORDER BY ... DESC on the TEXT column matches chronological order;
apply this change to all places that serialize these fields (the code paths
referencing run.started_at, run.created_at, run.updated_at, run.completed_at and
any insert/update helpers used by get_active_run and list queries).
- Around line 103-104: All write methods (save_run, update_run,
mark_interrupted, save_checkpoint, set_active, deactivate_all,
delete_checkpoint) must acquire a single shared async lock for the underlying
aiosqlite.Connection before starting execute()/commit() and perform a rollback
on error; implement a module-level mapping (e.g., a WeakKeyDictionary or attach
a lock attribute on the Connection keyed by the Connection object) so the same
lock is used across all repository instances that share the same db connection,
then wrap each write method with: await lock.acquire(); try: await
self._db.execute(...); await self._db.commit(); except Exception: await
self._db.rollback(); raise; finally: lock.release(); ensure imports for asyncio
and weakref (or equivalent) are added and apply this pattern to all seven DML
methods listed.

In `@web/src/pages/fine-tuning/PipelineControlPanel.tsx`:
- Around line 41-44: The component is directly calling
useFineTuningStore.setState({ preflight: null }) which bypasses the store's
coordinated actions and can create races; add a clearPreflight action to the
fine-tuning store (e.g., clearPreflight: () => set({ preflight: null, error:
null })) alongside existing runPreflightCheck, then replace the direct setState
call in PipelineControlPanel with the new action (invoke clearPreflight via
useFineTuningStore) so preflight is cleared through the store's action pattern.
- Around line 116-126: In PipelineControlPanel, remove the redundant aria-hidden
attribute from the advanced options container (the div with id
"advanced-options-panel") because the panel is conditionally rendered via
showAdvanced; keep the conditional rendering and ensure the toggle control's
aria-expanded and aria-controls attributes remain on the toggle button so
accessibility is preserved.

In `@web/src/pages/fine-tuning/RunHistoryTable.tsx`:
- Around line 39-40: The progress display uses run.stages_completed.length which
can include duplicates or non-active stages; instead compute the count of unique
stages that are present in ACTIVE_STAGES and render that. In
RunHistoryTable.tsx, replace usage of run.stages_completed.length with a derived
value (e.g., create a Set from run.stages_completed filtered by ACTIVE_STAGES
membership and take its size) so you count only unique, active stages before
outputting "{completedActiveCount}/{ACTIVE_STAGES.size}". Ensure you reference
run.stages_completed and ACTIVE_STAGES in the new computation.