feat(studio): KonfAI Studio web app over konfai-mcp, released in lockstep#65
Merged
Conversation
…udio feed A LiveControl channel steers lr and it_validation mid-run (recorded in the config as an audit trail), an on-demand validation pass runs on SIGUSR1, and a run stays alive when its console pipe closes -- the core hooks the Studio live feed drives.
Honour per-op tunables, keep an app run in the caller's session workspace (no throwaway sub-dir), add download_bundle (resolve + install into a directory), and refuse to fine-tune a config with no training loss.
Adds set_live_tunables, delete_run, request_validation and richer live_parse, and retires the app-execution wrappers in favour of import_app: a published app is copied into the session and run through the ordinary run_prediction / run_resume(weights_only) / run_evaluation tools.
Streams the chat over SSE, serves the front, streams volumes to NiiVue, and drives konfai-mcp through a pluggable brain (claude-code / openai / anthropic). A stream-broken agent is rebuilt and resumes its transcript.
…e wheel setup.py pins konfai-mcp to the exact release version (like apps/* pin konfai), and the package-data glob is recursive so web/assets/ (the JS/CSS/WASM) ships, not just web/index.html.
The build job runs npm ci && npm run build first (the React front is git-ignored) then python -m build --wheel (wheel-only: the sdist file-finder drops the built web/). Test job installs and smoke-tests studio.
brief-for-matt.html (a strategy pitch), PLAN.md (the build plan), and the spec-visual/ui-mockup HTML were dev artifacts, not part of the public package; the ops guide (REMOTE.md), the spec, and the screenshot stay.
Factor out the duplicated idioms the elegance review flagged, across the lockstep set. Behaviour preserved except the _experiment_info job ordering fix. - core: hoist the poll cadence into _LIVE_POLL_INTERVAL and fold the rank-0 produce+broadcast into _poll_from_rank0 (shared by both pollers) - apps: apply _clear_dataset in fine_tune teardown and add _collect_result, the mirror of _stage_result_dir, for infer/evaluate/uncertainty - mcp: drop the dead Job.set_parameters plumbing and the orphaned _parse_live_metric_line wrapper left by the live_parse extraction - studio BFF: _jail path guard (removes the drift across 5 sites), _all_jobs as the sole job.json reader (fixes _experiment_info ordering by mtime), _require_train_job, _apps_after, _mcp_detail/_mcp_json, _one_shot_claude - studio front: api.ts (getJson/postJson), useJson hook, readSSE, status.ts, and SPDX headers on the TS/TSX files
Break the ~1900-line server.py into focused modules and consolidate the front's per-session state. Behaviour preserved. - studio BFF: extract paths.py (jail + workspace/history), registry.py (a SessionState dataclass held in one dict[str, SessionState], replacing the 8 parallel dicts; sessions.json schema unchanged), auth.py, terminal.py, tensorboard.py, jobs.py. server.py (866 lines) is now app creation + route wiring; entangled experiment/job-control routes stay there by design. - studio front: consolidate nine per-session Record maps into one Record<string, SessionUiState> (sessionState.ts) with patchSession / replaceSessionField helpers. Verified: every route group serves 200 on relaunch, the registry round-trips the legacy sessions.json, the smoke tests pass, and tsc + build are clean.
Bring studio/konfai_studio under the same quality gates as the core: - pixi lint / format / format-check / typecheck now include studio - pre-commit ruff, ruff-format and mypy hooks now cover studio - fix the type smells this surfaces: SimpleCookie type-arg, an unannotated lambda -> functools.partial, an out/out_path type clash, and the str|None brain/model resolution narrowed via `x or default` - refresh the agent module docstring to list all three LLM backends (claude-code default, anthropic, openai) bandit stays scoped to core + apps: studio's findings are all Low (assert, try/except/pass, controlled TB/PTY subprocess) and would need per-line nosec noise for little value.
Wire konfai-mcp/konfai_mcp into the root ruff lint/format tasks and the pre-commit ruff/ruff-format hooks, matching studio and the core. The shared root [tool.ruff] config already governs it (line length 120, same rules), so this only aligns where the gates run, not the rules. Reformats the two files this surfaces (an import_app Annotated wrap and server_apps) that had drifted because konfai-mcp was outside the commit-time gate.
Bring apps and mcp under pixi typecheck + the mypy pre-commit hook, so all four packages are type-checked under the shared config. Fixes the errors this surfaces, type-only and behaviour-preserving: - apps: importlib spec guard, PackageMetadata read via subscript, explicit cast for the always-set upload group names, a typed files list (keeps the original single-zip dataset upload), gpu_list annotation - mcp: declare the host attributes MetricsServiceMixin borrows (TYPE_CHECKING stubs), guard the mean-delta operands, Message cast, isinstance guards - studio: guard repo._download behind isinstance(LocalAppRepository) -- a remote repo has no downloadable bundle, now a clean 404 Verified: pixi typecheck green across all four, the strict mypy hook passes --all-files, and the apps + mcp suites stay green (upload format unchanged).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughThis PR adds KonfAI Studio, replaces app-specific execution with import-based workflows, adds live training controls and log parsing, refactors fine-tuning artifacts, and updates packaging, documentation, tests, and release automation. ChangesKonfAI Studio
Import-Based App Workflow
Live Training and Fine-Tuning
Documentation and Release Wiring
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (3)
studio/konfai_studio/registry.py (1)
174-196: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGlobal
self._createlock serializes agent creation across all sessions.
agent()holds the single registry-wideself._createlock while awaitingmake_agent(...).__aenter__(), which spawns thekonfai-mcpsubprocess and completes the MCP handshake. A slow or hung spawn for one session blocks first-turn agent creation for every other session (head-of-line blocking), which works against the stated "run concurrently" design. Consider a per-session creation lock (or reuse the per-session turn lock) so building one task's agent can't stall others.Please confirm whether concurrent first-turns on distinct sessions are expected to build in parallel, or whether global serialization here is intentional to avoid a race in
make_agent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio/konfai_studio/registry.py` around lines 174 - 196, Replace the registry-wide self._create lock in agent() with per-session synchronization, reusing the session’s existing creation or turn lock if available, so stale-agent cleanup and make_agent(...).__aenter__() remain serialized only within the same session while distinct sessions can initialize concurrently. Preserve the existing state.agent reuse and stale reset behavior, and verify any shared make_agent race protection remains intact.konfai-apps/konfai_apps/app.py (2)
1518-1541: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSupport-file scan doesn't exclude
./Dataset(unlike the bundle-collection loop below).
Path(".").rglob("*")here walks the freshly-created./Dataset(line 1514) with no exclusion. Recursive**globbing normally skips symlinked directories, so this is harmless whenKonfAIApp.symlink()succeeds — but that helper falls back toshutil.copytree/copy2whenos.symlinkfails (Windows without Developer Mode, or a filesystem without symlink support), making./Dataseta real directory that IS traversed. On that path this loop walks the entire user dataset and can pick up/copy any incidentally-named.py/.yml/.yamlfile that happens to live inside it into the training scratch dir. The later bundle-collection loop (line 1601:skip = {"Dataset"}) was already hardened against exactly this; this loop should mirror it.♻️ Proposed fix
for support in Path(".").rglob("*"): - if not support.is_file() or "__pycache__" in support.parts: + if not support.is_file() or "__pycache__" in support.parts or "Dataset" in support.parts: continue🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@konfai-apps/konfai_apps/app.py` around lines 1518 - 1541, Exclude the top-level Dataset directory while scanning support files in the keep_training_artifacts branch. Update the Path(".").rglob("*") loop to skip entries whose path is rooted under Dataset, matching the existing bundle-collection skip behavior, while continuing to copy eligible support files elsewhere.
1367-1369: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valuePass
evaluations_dirby keyword here too.
Use the same explicit call style as the siblingevaluate()call to keep this independent of parameter order.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@konfai-apps/konfai_apps/app.py` around lines 1367 - 1369, Update the evaluate() call in the Uncertainties flow to pass evaluations_dir explicitly by keyword, matching the sibling evaluation call and avoiding dependence on positional parameter order.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.pre-commit-config.yaml:
- Around line 36-38: Update the file-matching regex for both the ruff and
ruff-format hooks to include the package-local test directories
konfai-apps/tests/, konfai-mcp/tests/, and studio/tests/, while preserving the
existing source and root tests/ matches.
In `@konfai-mcp/konfai_mcp/server.py`:
- Around line 2764-2785: Update request_validation to reject any job whose kind
is not "train" before checking active status or sending SIGUSR1, matching the
guard used by set_live_tunables. Preserve the existing response for valid
training jobs and return the appropriate failure response for non-training jobs.
In `@konfai/trainer.py`:
- Around line 233-237: Preload existing interventions from the config snapshot
when initializing _Trainer, so OOM-created instances preserve the audit trail
written by earlier retries. Update the initialization near _config_snapshot and
_interventions, and ensure _record_interventions continues from the loaded list
while retaining the existing overwrite behavior for the complete accumulated
history.
In `@studio/frontend/src/api.ts`:
- Around line 6-17: Update getJson and postJson to check the fetch Response.ok
status and throw for non-success HTTP responses before calling r.json().
Preserve JSON parsing for successful responses so callers can handle 401/500
failures through their existing catch paths.
In `@studio/frontend/src/Console.tsx`:
- Around line 44-51: Update the ws.onclose handler in the Console effect to
dispose the existing terminal instance before clearing references. Call
term.dispose(), then set termRef.current and fitRef.current to null so reopening
creates a clean terminal without duplicate DOM.
In `@studio/frontend/src/Deploy.tsx`:
- Around line 90-97: Update the run() flow in Deploy.tsx to track the backend
returned or selected during execution in a local variable, rather than using the
closed-over backend state value in the completion message. Use that local
backend variable in the setMessage interpolation while preserving the existing
setBackend state update.
In `@studio/frontend/src/FolderBrowser.tsx`:
- Around line 22-39: Update the FolderBrowser fetch flow to track an error
state, set a user-visible message when the `/api/browse` request fails or
parsing fails, and clear the error before each new request or after a successful
response. Render the error state near `modal-path` using the existing modal
error styling, while preserving the current directory and file updates on
success.
In `@studio/frontend/src/Login.tsx`:
- Around line 25-32: Update the session confirmation logic in the login flow to
stop converting getJson("/api/auth") failures into authenticated results. Let
the request rejection reach the existing error-handling path, and call
onAuthed() only when the actual response has authenticated set to true.
In `@studio/frontend/src/RightPanel.tsx`:
- Around line 1182-1186: Update the apply function to validate the numeric
results assigned to body.lr and body.it_validation, rejecting any non-finite
values before sending the request. Preserve the existing empty-input guard and
ensure invalid tunable text returns without proceeding to /api/job/tunables.
In `@studio/frontend/src/styles.css`:
- Around line 23-24: Update the --fsans and --fmono font-family values to
satisfy Stylelint’s value-keyword-case rule by lowercasing unquoted font names
or quoting proper/multi-word names; keep the intended fallback order and fonts
unchanged.
- Line 604: Replace deprecated word-break: break-word with overflow-wrap:
break-word in .tool pre, .console-body .ln, .toast, .app-name, and .feed-log .ln
at studio/frontend/src/styles.css lines 604, 1007, 1435, 2537, and 2962.
In `@studio/konfai_studio/tensorboard.py`:
- Around line 116-144: Expose a shutdown_servers() helper next to the
_TB_SERVERS cache that terminates each still-running TensorBoard process, safely
ignores termination errors, and clears the cache. Import and invoke
shutdown_servers() from server.py’s _lifespan finally block alongside await
_reg.close().
---
Nitpick comments:
In `@konfai-apps/konfai_apps/app.py`:
- Around line 1518-1541: Exclude the top-level Dataset directory while scanning
support files in the keep_training_artifacts branch. Update the
Path(".").rglob("*") loop to skip entries whose path is rooted under Dataset,
matching the existing bundle-collection skip behavior, while continuing to copy
eligible support files elsewhere.
- Around line 1367-1369: Update the evaluate() call in the Uncertainties flow to
pass evaluations_dir explicitly by keyword, matching the sibling evaluation call
and avoiding dependence on positional parameter order.
In `@studio/konfai_studio/registry.py`:
- Around line 174-196: Replace the registry-wide self._create lock in agent()
with per-session synchronization, reusing the session’s existing creation or
turn lock if available, so stale-agent cleanup and make_agent(...).__aenter__()
remain serialized only within the same session while distinct sessions can
initialize concurrently. Preserve the existing state.agent reuse and stale reset
behavior, and verify any shared make_agent race protection remains intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08ac53c2-5f35-4d6d-89a4-51f2e269f362
⛔ Files ignored due to path filters (6)
docs/source/_static/konfai-studio.pngis excluded by!**/*.pngstudio/docs/screenshot.pngis excluded by!**/*.pngstudio/frontend/package-lock.jsonis excluded by!**/package-lock.jsonstudio/frontend/src/konfai-rs/konfai_rs_bg.wasmis excluded by!**/*.wasmstudio/konfai_studio/web/konfai-logo-light.pngis excluded by!**/*.pngstudio/konfai_studio/web/konfai-logo.pngis excluded by!**/*.png
📒 Files selected for processing (87)
.claude/skills/konfai-experiments/SKILL.md.claude/skills/konfai-experiments/references/tool-reference.md.github/workflows/publish.yml.pre-commit-config.yamlAGENTS.mdREADME.mddocs/source/index.rstdocs/source/usage/index.rstdocs/source/usage/mcp.mddocs/source/usage/studio.mdkonfai-apps/konfai_apps/app.pykonfai-apps/konfai_apps/app_repository.pykonfai-apps/konfai_apps/app_server.pykonfai-apps/tests/unit/test_finetune_lr_override.pykonfai-apps/tests/unit/test_finetune_requires_loss.pykonfai-mcp/README.mdkonfai-mcp/konfai_mcp/capabilities.pykonfai-mcp/konfai_mcp/extensions.pykonfai-mcp/konfai_mcp/guide.pykonfai-mcp/konfai_mcp/live_parse.pykonfai-mcp/konfai_mcp/metrics_service.pykonfai-mcp/konfai_mcp/runner.pykonfai-mcp/konfai_mcp/server.pykonfai-mcp/konfai_mcp/server_apps.pykonfai-mcp/konfai_mcp/server_jobs.pykonfai-mcp/konfai_mcp/server_support.pykonfai-mcp/konfai_mcp/workflows.pykonfai-mcp/tests/test_live_parse.pykonfai-mcp/tests/test_mcp_server_apps.pykonfai-mcp/tests/test_mcp_server_delete_run.pykonfai-mcp/tests/test_mcp_server_fixes.pykonfai-mcp/tests/test_mcp_server_refine_loop.pykonfai-mcp/tests/test_mcp_server_set_live_tunables.pykonfai-mcp/tests/test_mcp_server_tool_index.pykonfai-mcp/tests/test_review_mcp_apps_misc.pykonfai-mcp/tests/test_workflow_registry.pykonfai/network/network.pykonfai/trainer.pykonfai/utils/live_control.pykonfai/utils/runtime.pypyproject.tomlstudio/.gitignorestudio/LICENSEstudio/README.mdstudio/docs/REMOTE.mdstudio/docs/STUDIO_SPEC.mdstudio/frontend/.gitignorestudio/frontend/index.htmlstudio/frontend/package.jsonstudio/frontend/src/App.tsxstudio/frontend/src/AppZoo.tsxstudio/frontend/src/Chat.tsxstudio/frontend/src/Console.tsxstudio/frontend/src/Deploy.tsxstudio/frontend/src/FolderBrowser.tsxstudio/frontend/src/Login.tsxstudio/frontend/src/RightPanel.tsxstudio/frontend/src/Viewer.tsxstudio/frontend/src/api.tsstudio/frontend/src/konfai-rs/konfai_rs.d.tsstudio/frontend/src/konfai-rs/konfai_rs.jsstudio/frontend/src/konfai-rs/konfai_rs_bg.wasm.d.tsstudio/frontend/src/main.tsxstudio/frontend/src/sessionState.tsstudio/frontend/src/sse.tsstudio/frontend/src/status.tsstudio/frontend/src/styles.cssstudio/frontend/src/useJobStream.tsstudio/frontend/src/useJson.tsstudio/frontend/tsconfig.jsonstudio/frontend/vite.config.tsstudio/konfai_studio/__init__.pystudio/konfai_studio/__main__.pystudio/konfai_studio/agent.pystudio/konfai_studio/auth.pystudio/konfai_studio/cli.pystudio/konfai_studio/jobs.pystudio/konfai_studio/paths.pystudio/konfai_studio/registry.pystudio/konfai_studio/server.pystudio/konfai_studio/tensorboard.pystudio/konfai_studio/terminal.pystudio/pyproject.tomlstudio/setup.pystudio/tests/test_smoke.pytests/unit/test_live_control.pytests/unit/test_trainer.py
💤 Files with no reviewable changes (2)
- konfai-mcp/tests/test_mcp_server_refine_loop.py
- konfai-mcp/tests/test_review_mcp_apps_misc.py
Comment on lines
+36
to
+38
| files: ^(konfai/|konfai-apps/konfai_apps/|konfai-mcp/konfai_mcp/|studio/konfai_studio/|tests/) | ||
| - id: ruff-format | ||
| files: ^(konfai/|konfai-apps/konfai_apps/|tests/) | ||
| files: ^(konfai/|konfai-apps/konfai_apps/|konfai-mcp/konfai_mcp/|studio/konfai_studio/|tests/) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== .pre-commit-config.yaml relevant lines ==\n'
nl -ba .pre-commit-config.yaml | sed -n '28,50p'
printf '\n== test directories in repo ==\n'
git ls-files | awk '
/(^|\/)tests\// { print }
' | sort
printf '\n== package-local test paths presence checks ==\n'
for p in 'konfai-apps/tests' 'konfai-mcp/tests' 'studio/tests' 'tests'; do
printf '%s: ' "$p"
if git ls-files "$p" | grep -q .; then
echo present
git ls-files "$p" | sed -n '1,20p'
else
echo absent
fi
doneRepository: fideus-labs/KonfAI
Length of output: 242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== .pre-commit-config.yaml relevant lines ==\n'
awk 'NR>=28 && NR<=50 { printf "%4d %s\n", NR, $0 }' .pre-commit-config.yaml
printf '\n== test directories in repo ==\n'
git ls-files | awk '
/(^|\/)tests\// { print }
' | sort
printf '\n== package-local test paths presence checks ==\n'
for p in 'konfai-apps/tests' 'konfai-mcp/tests' 'studio/tests' 'tests'; do
printf '%s: ' "$p"
if git ls-files "$p" | grep -q .; then
echo present
git ls-files "$p" | sed -n '1,20p'
else
echo absent
fi
doneRepository: fideus-labs/KonfAI
Length of output: 9394
Include package-local test trees in Ruff checks.
The regex matches the root tests/ tree, but not konfai-apps/tests/, konfai-mcp/tests/, or studio/tests/, so those tests bypass ruff and ruff-format.
Proposed fix
- files: ^(konfai/|konfai-apps/konfai_apps/|konfai-mcp/konfai_mcp/|studio/konfai_studio/|tests/)
+ files: ^(konfai/|konfai-apps/(konfai_apps/|tests/)|konfai-mcp/(konfai_mcp/|tests/)|studio/(konfai_studio/|tests/)|tests/)Apply the same pattern to ruff-format.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.pre-commit-config.yaml around lines 36 - 38, Update the file-matching regex
for both the ruff and ruff-format hooks to include the package-local test
directories konfai-apps/tests/, konfai-mcp/tests/, and studio/tests/, while
preserving the existing source and root tests/ matches.
Comment on lines
+2764
to
+2785
| def request_validation( | ||
| kind: Annotated[ | ||
| WorkflowKind, Field(description="Job kind to target when job_id is omitted (default 'train').") | ||
| ] = "train", | ||
| job_id: Annotated[ | ||
| str | None, Field(description="Exact job to validate (default: the latest job of the given kind).") | ||
| ] = None, | ||
| ) -> dict[str, Any]: | ||
| """Ask a running training job to run a validation pass now, via SIGUSR1.""" | ||
| job = JOB_REGISTRY.get(job_id) if job_id is not None else SESSION.discover_latest_job(kind) | ||
| if job is None or job.status not in ACTIVE_JOB_STATES: | ||
| return {"ok": False, "detail": "No running training job to validate."} | ||
| delivered = JOB_REGISTRY.notify(job, signal.SIGUSR1) | ||
| return { | ||
| "ok": delivered, | ||
| "job_id": job.job_id, | ||
| "detail": ( | ||
| "Validation requested; it runs at the next iteration boundary." | ||
| if delivered | ||
| else "Could not signal the job (already finished, or not signalable)." | ||
| ), | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Where SIGUSR1 is handled across the runtimes.
rg -nP 'SIGUSR1|signal\.signal|sigaction' konfai konfai-mcp -C3Repository: fideus-labs/KonfAI
Length of output: 6878
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant job-discovery and signal-handling code.
sed -n '1,220p' konfai-mcp/konfai_mcp/server_jobs.py
printf '\n--- server.py around request_validation ---\n'
sed -n '2748,2798p' konfai-mcp/konfai_mcp/server.py
printf '\n--- server.py around set_live_tunables ---\n'
sed -n '2798,2860p' konfai-mcp/konfai_mcp/server.pyRepository: fideus-labs/KonfAI
Length of output: 14574
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check how jobs are launched and how latest-job selection works.
rg -n "_run_job\(" konfai-mcp/konfai_mcp/server.py konfai-mcp/konfai_mcp/server_jobs.py
printf '\n--- discover_latest_job ---\n'
sed -n '560,760p' konfai-mcp/konfai_mcp/server_jobs.pyRepository: fideus-labs/KonfAI
Length of output: 8395
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect WorkflowKind and latest-job selection semantics.
rg -n "class WorkflowKind|WorkflowKind =|discover_latest_job|active_states|kind ==" konfai-mcp/konfai_mcp/server.py konfai-mcp/konfai_mcp/server_jobs.py konfai-mcp/konfai_mcp/workflows.py -C3Repository: fideus-labs/KonfAI
Length of output: 7648
Restrict request_validation to training jobs SIGUSR1 is ignored by the job wrapper for non-training runs, so this can return success without triggering validation. Mirror set_live_tunables and reject non-train jobs up front.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@konfai-mcp/konfai_mcp/server.py` around lines 2764 - 2785, Update
request_validation to reject any job whose kind is not "train" before checking
active status or sending SIGUSR1, matching the guard used by set_live_tunables.
Preserve the existing response for valid training jobs and return the
appropriate failure response for non-training jobs.
Comment on lines
+44
to
+51
| ws.onclose = () => { | ||
| setConnected(false); | ||
| term.write("\r\n\x1b[2m[shell ended — reopen the drawer to start a new one]\x1b[0m\r\n"); | ||
| termRef.current = null; // let a reopen spawn a fresh shell | ||
| }; | ||
| term.onData((d) => ws.readyState === WebSocket.OPEN && ws.send(JSON.stringify({ type: "input", data: d }))); | ||
| term.onResize(({ cols, rows }) => ws.readyState === WebSocket.OPEN && ws.send(JSON.stringify({ type: "resize", cols, rows }))); | ||
| }, [open]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and inspect the relevant region with line numbers.
git ls-files studio/frontend/src/Console.tsx
wc -l studio/frontend/src/Console.tsx
cat -n studio/frontend/src/Console.tsx | sed -n '1,220p'
# Search for terminal lifecycle handling in the frontend.
rg -n "term\.dispose|hostRef|termRef|ws\.onclose|fitRef|term\.open|new Terminal|new FitAddon" studio/frontend/src -SRepository: fideus-labs/KonfAI
Length of output: 5475
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Identify the xterm package version and any local cleanup patterns.
rg -n '"`@xterm/xterm`"|"`@xterm/addon-fit`"|xterm' studio/frontend/package.json studio/frontend/package-lock.json studio/frontend/yarn.lock package.json package-lock.json yarn.lock -S
# Inspect any docs or comments around the console/terminal behavior.
rg -n "dispose\\(|open\\(hostRef|term\\.open|term\\.dispose|shell ended|reopen the drawer" studio/frontend/src -S
# Read the frontend package manifest if present.
if [ -f studio/frontend/package.json ]; then
cat -n studio/frontend/package.json
fiRepository: fideus-labs/KonfAI
Length of output: 1052
🌐 Web query:
xterm.js Terminal open dispose host container DOM append behavior dispose removes terminal DOM open same host without dispose
💡 Result:
In xterm.js, the terminal.dispose() method is designed to be a terminal operation for a terminal instance [1][2]. When called, dispose() removes the terminal's main DOM element from the parent container [3], cleans up internal event listeners, and releases references to prevent memory leaks [4][5]. Crucially, once dispose() has been called, the terminal instance should be considered unusable [6][1][7]. Attempting to reuse the same instance (e.g., calling open() again on a disposed terminal) is unsupported and likely to cause errors or broken behavior, such as missing render cycles or unresponsive event handlers [1][8][2]. If you need to "reset" or "re-open" a terminal in the same DOM container: 1. Do not attempt to reuse the disposed instance. 2. Instead, create a new Terminal instance using new Terminal() [2]. 3. Manually ensure the previous terminal's container is clear (if necessary), then call open(parent) with the new instance [1]. While some users have historically attempted to work around this by manually managing DOM elements or patching internal states, the recommended and stable pattern is to treat the Terminal object as a single-use lifecycle object that is destroyed and recreated when needed [1][2].
Citations:
- 1: Should
.dispose()be a trapdoor function? xtermjs/xterm.js#3939 - 2: re-create term obj ,no mouse event repsoned. Is there some re-enter limitation existed in xterm.js xtermjs/xterm.js#5161
- 3: https://github.com/xtermjs/xterm.js/blob/d3fd3d0f2254a966238e575f3505d5cac31cff91/src/browser/Terminal.ts
- 4: Ensure all references are released when Terminal.dispose is called xtermjs/xterm.js#1525
- 5: Three dispose-registration gaps leak Terminal instances past host unmount xtermjs/xterm.js#5818
- 6: https://github.com/xtermjs/xterm.js/blob/6.0.0/typings/xterm.d.ts
- 7: Clarify you shouldn't use the object after Terminal.dispose xtermjs/xterm.js#3958
- 8: The terminal instance cannot be rendered correctly after calling
.open()for the second time. xtermjs/xterm.js#4978
Dispose the terminal on ws.onclose. termRef.current = null alone leaves the old xterm instance mounted; reopening the drawer creates a new terminal in the same host and can stack duplicate DOM. Call term.dispose() there, then clear termRef/fitRef.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@studio/frontend/src/Console.tsx` around lines 44 - 51, Update the ws.onclose
handler in the Console effect to dispose the existing terminal instance before
clearing references. Call term.dispose(), then set termRef.current and
fitRef.current to null so reopening creates a clean terminal without duplicate
DOM.
Comment on lines
+25
to
+32
| // Confirm the session cookie actually stuck. Over plain http a Secure cookie is silently dropped, | ||
| // which would otherwise bounce straight back to this screen with no explanation. | ||
| const st = await getJson<{ authenticated?: boolean }>("/api/auth").catch(() => ({ authenticated: true })); | ||
| if (st.authenticated) { | ||
| onAuthed(); | ||
| return; | ||
| } | ||
| setErr("Signed in, but the session cookie wasn't stored — are you on HTTPS? (see docs/REMOTE.md)"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not treat a failed session check as authenticated.
The fallback returns authenticated: true when confirmation fails, so onAuthed() runs without verifying the cookie. Let the request failure reach the existing error handler instead.
Proposed fix
- const st = await getJson<{ authenticated?: boolean }>("/api/auth").catch(() => ({ authenticated: true }));
+ const st = await getJson<{ authenticated?: boolean }>("/api/auth");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Confirm the session cookie actually stuck. Over plain http a Secure cookie is silently dropped, | |
| // which would otherwise bounce straight back to this screen with no explanation. | |
| const st = await getJson<{ authenticated?: boolean }>("/api/auth").catch(() => ({ authenticated: true })); | |
| if (st.authenticated) { | |
| onAuthed(); | |
| return; | |
| } | |
| setErr("Signed in, but the session cookie wasn't stored — are you on HTTPS? (see docs/REMOTE.md)"); | |
| // Confirm the session cookie actually stuck. Over plain http a Secure cookie is silently dropped, | |
| // which would otherwise bounce straight back to this screen with no explanation. | |
| const st = await getJson<{ authenticated?: boolean }>("/api/auth"); | |
| if (st.authenticated) { | |
| onAuthed(); | |
| return; | |
| } | |
| setErr("Signed in, but the session cookie wasn't stored — are you on HTTPS? (see docs/REMOTE.md)"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@studio/frontend/src/Login.tsx` around lines 25 - 32, Update the session
confirmation logic in the login flow to stop converting getJson("/api/auth")
failures into authenticated results. Let the request rejection reach the
existing error-handling path, and call onAuthed() only when the actual response
has authenticated set to true.
Comment on lines
+116
to
+144
| @router.get("/api/tensorboard") | ||
| async def tensorboard_link(session: str = Query("default")) -> dict[str, Any]: | ||
| """Ensure a TensorBoard server is running for the task's Statistics dir and return its URL — the full | ||
| TensorBoard UI (scalars, images, histograms) for every run, alongside Studio's own live feed. One | ||
| server per task, reused while alive, bound to 127.0.0.1 (a remote deployment must proxy the port).""" | ||
| name = _sane_session(session) | ||
| live = _TB_SERVERS.get(name) | ||
| if live and live["proc"].poll() is None: | ||
| return {"ok": True, "url": live["url"]} | ||
| # The session root, not Statistics/: TensorBoard recurses, so it surfaces every "*/tb" at any depth — | ||
| # session-root runs AND isolated app-output runs (<app_output>-<hash>/Statistics/<run>/tb) alike. | ||
| session_root = _workspace_root() / "sessions" / name | ||
| if not session_root.is_dir() or not any(session_root.glob("**/tb")): | ||
| return {"ok": False, "detail": "no TensorBoard events yet — run a training first"} | ||
| # Look next to the running interpreter first: the server is launched as a console script, so the env's | ||
| # bin dir may not be on PATH and shutil.which alone would miss it. | ||
| sibling = Path(sys.executable).with_name("tensorboard") | ||
| binary = str(sibling) if sibling.exists() else shutil.which("tensorboard") | ||
| if not binary: | ||
| return {"ok": False, "detail": "tensorboard is not installed (pip install konfai[tensorboard])"} | ||
| port = _free_port() | ||
| proc = subprocess.Popen( | ||
| [binary, "--logdir", str(session_root), "--host", "127.0.0.1", "--port", str(port)], | ||
| stdout=subprocess.DEVNULL, | ||
| stderr=subprocess.DEVNULL, | ||
| ) | ||
| url = f"http://127.0.0.1:{port}/" | ||
| _TB_SERVERS[name] = {"proc": proc, "url": url} | ||
| return {"ok": True, "url": url} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Cached TensorBoard subprocesses are never reaped on shutdown.
_TB_SERVERS caches one long-lived tensorboard subprocess per session, but nothing terminates them — server.py's _lifespan only closes _reg. On Studio shutdown/restart these processes are orphaned, keep holding their 127.0.0.1 ports and memory, and accumulate across restarts.
Consider exposing a reaper and wiring it into the app lifespan.
🧹 Suggested reaper + lifespan wiring
In tensorboard.py:
def shutdown_servers() -> None:
"""Terminate all cached TensorBoard subprocesses."""
import signal
for entry in _TB_SERVERS.values():
proc = entry.get("proc")
if proc is not None and proc.poll() is None:
with suppress(Exception):
proc.send_signal(signal.SIGTERM)
_TB_SERVERS.clear()Then call shutdown_servers() from server.py's _lifespan finally block alongside await _reg.close().
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 136-140: Command coming from incoming request
Context: subprocess.Popen(
[binary, "--logdir", str(session_root), "--host", "127.0.0.1", "--port", str(port)],
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@studio/konfai_studio/tensorboard.py` around lines 116 - 144, Expose a
shutdown_servers() helper next to the _TB_SERVERS cache that terminates each
still-running TensorBoard process, safely ignores termination errors, and clears
the cache. Import and invoke shutdown_servers() from server.py’s _lifespan
finally block alongside await _reg.close().
- core: SIGUSR1 does not exist on Windows, so installing the handler raised AttributeError (past the ValueError/OSError suppress) and broke every _Trainer construction there -- this is what turned the Windows CI red. getattr-guard it, and the mcp job wrapper's SIG_IGN the same way. - core: merge the intervention audit trail into the config snapshot instead of overwriting it, so an OOM auto-restart no longer wipes the interventions the previous instance had already recorded. - studio BFF: reap the cached TensorBoard subprocesses on app shutdown; they were orphaned across restarts, holding their ports. - studio front: throw on non-ok responses in getJson/postJson; never treat a failed session check as authenticated; report the backend actually chosen in the deploy done-message; surface /api/browse failures; reject non-finite tunables; quote font names and replace deprecated word-break: break-word.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds KonfAI Studio — a FastAPI BFF + built React front over
konfai-mcp— as thekonfai-studiopackage, taking the release matrix to 9 packages. A clinician-researcher points at a dataset and drives
train / infer / visualize / reproduce from one chat surface; the LLM brain is pluggable (Claude Code /
Anthropic / local OpenAI-compatible via
KONFAI_STUDIO_LLM), and data never leaves the machine.Why this is one lockstep PR
konfai_studio.serverimportskonfai_mcp.live_parse, so Studio and konfai-mcp must ship the sameversion. This PR therefore lands the coupled set for v1.6.1:
download_bundle(
import_app→run_prediction/run_resume/run_evaluation) + testsCI matrix entry, Apache-2.0 LICENSE
Quality / consistency
(net line reduction; adds
_jail,_all_jobs,_mcp_detail, frontapi.ts/useJson/readSSE, …),and fixed a latent job-ordering bug (
_experiment_infosorted by mtime).server.py(1900 → 866 lines) into focused modules (paths,registrywith aSessionStatedataclass,auth,terminal,tensorboard,jobs);App.tsxper-session stateconsolidated into one
Record<string, SessionUiState>.studioandkonfai-mcpare now under the shared root ruff + mypy gates(pixi tasks + pre-commit hooks). All four packages share one
[tool.ruff]config and type-check clean;wiring them in surfaced and fixed real drift (unformatted mcp files, a cross-package
AppRepositoryInfo._downloadaccess in studio). bandit stays scoped to core + apps.Verification
pixi typecheckgreen across all four packages; the strict mypy pre-commit hook passes--all-filesmain; wheel ships the built React front (package-data = web/**/*),konfai-mcp=={version}lockstep pin, tag-derived versionRelease
After merge, tag
v1.6.1→publish.ymlbuilds + publishes all 9 packages (Studio front built via Node inCI) to PyPI over OIDC, then the Docker image. The
konfai-studioPyPI trusted-publisher is registered.Summary by CodeRabbit
New Features
Bug Fixes
Documentation