fix(frontend): dedupe EventSource — share one session SSE stream - #148
Closed
lucastononro wants to merge 110 commits into
Closed
fix(frontend): dedupe EventSource — share one session SSE stream#148lucastononro wants to merge 110 commits into
lucastononro wants to merge 110 commits into
Conversation
Adds `GET /api/sessions/{sid}/download` and `GET /api/projects/{pid}/download`
that stream a self-contained zip of the agent's workspace: every file under
`/sessions/{sid}/` plus a synthetic `trainable_local.py` shim, a filtered
`requirements.txt`, and a runbook README. The shim lets downloaded scripts
that import `from trainable import log, log_image, ...` run on a vanilla
Python install — calls land in `./trainable_out/` instead of the Modal
volume.
The zip is streamed via `StreamingResponse` over an in-memory buffer
drained per write, so multi-hundred-MB sessions don't materialize on
disk or in RAM. A 2 GB uncompressed cap with a trailing `__truncated.txt`
marker keeps a runaway walk bounded.
Frontend ships two entry points:
- Download icon in the WorkspaceSidebar header → session zip
- Download icon on every Project row in the sidebar → project zip
Closes #79.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The cli/README.md was stale against v0.0.4: it described a single-provider (Anthropic + Modal) setup, claimed the wizard downloaded the compose file from raw.githubusercontent.com, and omitted `trainable reconfigure`. Update it to match the current implementation: - multi-provider selection (Claude API key / subscription OAuth, OpenAI, Gemini, LiteLLM-routed backends) as a flat picker, with a Providers table - compose file bundled inside the wheel (offline installs) - version-pinned images on `trainable up` - `trainable reconfigure` command and the existing-config merge flow Assisted-by: Claude Code
…#123) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
…tores (#90) docker-compose.prod.yml shipped hardcoded default credentials (trainable/trainable, minioadmin/minioadmin) and published Postgres (5432), MinIO (9000/9001) and the auth-less Jaeger UI (16686) on 0.0.0.0 — direct data-store compromise with well-known creds on any networked host, and Jaeger traces leak prompts/paths/token counts. - Replace every hardcoded secret with required env-var interpolation that fails loudly if unset: POSTGRES_USER/PASSWORD/DB and MINIO_ROOT_USER/PASSWORD via `${VAR:?set VAR in .env}`. The backend's DATABASE_URL and AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY are now derived from those same vars, so app creds always match the datastores (var names match what backend/config.py reads). - Bind Postgres, MinIO API+console and the Jaeger UI to 127.0.0.1 so they are reachable only from the host / internal compose network, never from another machine. Drop the Jaeger OTLP host publishes (4317/4318) — the backend reaches them by service name internally. frontend (3000) and backend (8000) stay published as before. - Document the newly-required vars in .env.example with placeholders and a note that prod has no defaults. Validated: `docker compose -f docker-compose.prod.yml config` parses and interpolates with a filled .env, and fails fast with a clear message when the secrets are unset. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
Add an opt-in shared-secret gate for the API. When API_AUTH_TOKEN is
unset (the default) nothing changes — every endpoint stays open, so
local dev and the frontend are unaffected. When set, all /api/*
requests must send `Authorization: Bearer <token>`.
- backend/auth.py: pure-ASGI BearerTokenAuthMiddleware (no
BaseHTTPMiddleware wrapping, so SSE streaming is untouched);
constant-time token comparison; 401 + WWW-Authenticate on failure.
- Exempt: /api/health, /api/readyz, CORS preflight (OPTIONS).
- SSE: EventSource cannot send headers, so the stream endpoint
(/api/sessions/{id}/stream) additionally accepts ?token=<token>;
the query param is stream-only and does not unlock other routes.
- config.py: api_auth_token setting (env API_AUTH_TOKEN, default None).
- Middleware added before CORSMiddleware so CORS stays outermost and
answers preflights before auth runs.
- .env.example: new "Optional — security" section documenting it.
- tests/test_api_auth.py: middleware unit tests (open-by-default,
401/200 matrix, exemptions, stream query token, preflight).
Closes #88
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
…edentials Replace the `cors_origins=["*"]` + `allow_credentials=True` default (any web page could script credentialed cross-origin requests against localhost:8000) with a safe, configurable policy: - config.py: default origins are the local frontend (http://localhost:3000, http://127.0.0.1:3000); overridable via CORS_ORIGINS as a comma-separated env var (NoDecode + validator so plain CSV works, not just JSON). - main.py: if `*` is configured it is honored but credentials are disabled (allow_credentials=False) with a startup warning — the wildcard+credentials combination is now impossible. - .env.example: document CORS_ORIGINS. No impact on the standard setup: the frontend proxies /api through Next.js rewrites (same-origin), so CORS is not involved there. Closes #89 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
- Stream uploads to S3 in bounded 8 MB chunks (single put_object for small bodies, multipart upload for larger ones) instead of reading the whole body into RAM; abort the multipart upload on any failure. - Enforce the configured max_upload_size_bytes cap incrementally while reading (413 without buffering the oversize body). - Validate bucket against the app-provisioned allowlist (new settings.s3_allowed_buckets, also used by startup bucket init) and reject traversal/absolute/backslash keys; write endpoints (upload + put-presign) are scoped to the datasets/projects/ prefix. - Authentication is intentionally NOT added here — it lands globally via issue #88. Closes #91 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
A stalled provider HTTP call had no wall-clock limit: the runner's outer asyncio.timeout was deliberately removed (the Modal sandbox timeout only bounds tool execution), and every provider accepted timeout_seconds but discarded it. A hung network call left the session's background task alive forever — spinner never cleared, task-registry entry never freed. Thread the existing timeout_seconds into each provider around the HTTP call only, so tool-execution time is never counted: - base.py: new enforce_wall_clock() helper — asyncio.wait_for around a single provider call, raising builtin TimeoutError on expiry. - openai: per-request SDK timeout via client.with_options(timeout=...) plus the hard wall-clock cap (SDK retries can't stretch past it). - gemini: hard cap around generate_content (google-genai has no default request timeout at all). - litellm: keep the existing per-attempt timeout= knob, add the hard cap so backend retries/fallbacks can't exceed the budget. - claude: query() runs the whole tool loop internally, so it must not be wrapped wholesale; bound each underlying API HTTP request via Claude Code's API_TIMEOUT_MS env instead (caller-supplied value wins). On expiry the providers let TimeoutError propagate: run_agent's existing TimeoutError handler publishes agent_timeout, moves the session to timed_out, and the finally block frees the task registry — the run ends cleanly instead of hanging. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
Every boto3 call ran synchronously inside async def handlers/services, stalling the event loop for each S3 round-trip. Wrap them all in asyncio.to_thread, behavior-identical: - services/s3_sync.py: put_object in the per-file sync loop - routers/s3_browser.py: list_buckets, list_objects_v2, presign generation, and the whole upload path (put_object + multipart create/upload_part/complete/abort) - routers/experiments.py: put_object in create_experiment/attach_data; the four get_object(...).read() sites now stream to a temp file in bounded 1 MB chunks inside a worker thread (_download_to_tempfile) instead of reading whole bodies into memory on the loop Test mock: conftest get_object now returns a fresh BytesIO body per call so chunked reads hit EOF like a real StreamingBody. Closes #92 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
/api/health stays a cheap static liveness check. New /api/readyz pings the DB (SELECT 1 on the async engine) and S3 (list_buckets via asyncio.to_thread so sync boto3 doesn't block the event loop) and returns 503 with per-check detail when either is down, so an orchestrator no longer routes traffic to a backend whose Postgres pool or object store is dead. Compose wiring (both docker-compose.yml and docker-compose.prod.yml): - backend healthcheck: curl -fsS /api/health (curl is already in the backend image), 5s interval, 15s start_period - frontend depends_on backend with condition: service_healthy, so the frontend waits for a responsive backend instead of racing it tests/test_readyz.py covers ready, S3-down, DB-down, and health-stays- static; verified live in-process (real app, TestClient): 200 when both up, 503 EndpointConnectionError when S3 unreachable, 503 when DB connect fails. Both compose files pass `docker compose config -q`. Closes #119 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
create_experiment accumulated every file fully in RAM (content += chunk) and then retained all bytes again in the staged tuples, so a folder upload of many large files (500 MB per-file cap) could OOM the worker. - Stream each multipart body straight to its temp file in bounded 1 MB chunks; the size cap is enforced on a running counter and the content hash for dataset versioning is computed incrementally (sha256) while streaming — the full bytes are never materialized. - Staged tuples are now (tmp_path, remote_path) only; the temp path is registered before streaming so the finally block also cleans up partially-written files on mid-stream failure. - The S3 push now uses s3.upload_file from the temp file (streams from disk, off-loop) instead of put_object(Body=<whole bytes>). - record_upload accepts precomputed content_hash/size_bytes so streaming callers don't need the full bytes; content-based callers unchanged. Closes #94 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
HomePage.connectSSE and useNotebookSSE each opened their own EventSource
to the identical /api/sessions/{id}/stream endpoint, doubling server
fan-out and parsing every event twice. Add SSEStreamContext to expose
the single page-level stream via a publish/subscribe bus; the notebook
now subscribes instead of opening a second connection.
Closes #100
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
Adds pytest-cov to backend/requirements.txt and runs the suite with --cov=. --cov-report=xml --cov-report=term-missing, uploading the coverage.xml as a build artifact. Sets --cov-fail-under=60 as a floor (measured baseline is ~66%), catching future regressions in untested modules while leaving headroom. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
) generic_exception_handler only logged unhandled request exceptions and never reported them to Sentry. Background agent work spawned via asyncio.create_task in routers/sessions.py (send_message's _run_followup) runs outside the request lifecycle, so FastAPI's exception handler never saw those errors either — they were silently swallowed into a "failed" session state with no logging or tracking. Add errors.capture_exception(), a thin no-op-safe wrapper around sentry_sdk.capture_exception (no-ops with no DSN and never lets a Sentry-side failure mask the original error), call it from the generic handler, and call it (plus add the missing logger.exception) from the background-task except block. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
…grations) (#121) Scaffolds backend/alembic/ wired to the app's async engine (settings.database_url) and Base.metadata, with a single autogenerated initial revision reflecting today's full schema. init_db() now runs `alembic upgrade head` on boot (in a worker thread, since env.py drives its own asyncio.run()); a DB that already has the full schema but no alembic_version table gets `stamp head` instead, so existing deployments don't get destructive re-creation attempts. _run_migrations is kept in place but unused, documented as dead code for rollback reference. Verified schema-equivalence of `alembic upgrade head` (fresh DB) vs `create_all + _run_migrations` (legacy path) on both SQLite and Postgres: only difference found is two redundant duplicate indexes the legacy code created under typo'd names, no logical schema divergence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
backend-test now spins up a postgres:16-alpine service container and points TEST_DATABASE_URL at it; conftest.py respects TEST_DATABASE_URL (falling back to DATABASE_URL, then the sqlite default for local dev) instead of unconditionally forcing sqlite+aiosqlite. This exercises the Column(JSON) fields, FK ON DELETE CASCADE behavior, and the hand-rolled db._run_migrations against the engine we actually ship. Also pins asyncio_default_fixture_loop_scope/asyncio_default_test_loop_scope to "session" in a new backend/pytest.ini. Without it, pytest-asyncio 1.x's per-test event loop (fine for aiosqlite) breaks the module-level asyncpg engine with "Future attached to a different loop" once a second test needs the DB. Pinning both to session fixes this for both SQLite and Postgres. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
Adds focused unit tests for the two most critical, near-untested modules in the agent runtime, with the Modal boundary faked throughout: - test_sandbox.py: run_code's success/timeout/exception paths against a faked modal.Sandbox (create/stdout/stderr/wait) — asserts span attributes, metric/log-event dispatch routing, and usage recording per path. - test_runner.py: run_agent's reachable orchestration state machine (spawn -> run -> complete/error/cancel) and its interplay with the real task registry (register_task/abort_agent/is_agent_running/ cleanup_session), using real asyncio Tasks for the registry tests so cancellation semantics are genuine. _drive_provider is faked — its internals are already covered by test_drive_provider.py. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
CI's only security step was bandit (Python SAST) — no dependency
scanning and no image scanning, despite requirements.txt already
carrying a manual note pinning litellm to dodge a CVE that a scanner
would enforce automatically.
Adds three new jobs, all advisory (continue-on-error: true on the
scan step) so pre-existing advisories don't break CI:
- backend-vuln-scan: pip-audit against backend/requirements.txt
- frontend-vuln-scan: npm audit against the frontend lockfile
- image-scan: builds trainable-backend/trainable-frontend locally
(not pushed) and scans them with Trivy (CRITICAL/HIGH)
Each can be flipped to blocking later by dropping continue-on-error
once its current advisories are triaged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
The backend /compare endpoint already aggregates metrics, feature overlap, and cost totals across up to 8 sessions, but nothing in the frontend consumed it. This adds the leaderboard UI: - api.ts: typed api.compare(sessionIds) client fn; types.ts: Compare* types mirroring routers/compare.py (incl. the feature_overlap list-when-empty quirk) - /compare page: sortable leaderboard table (per-metric latest values with best-value highlight, total cost, created-at), overlaid per-metric charts reusing the MetricsTab chart stack (palette, tooltip, formatters now exported), session legend toggles, and a feature-overlap panel (common + per-session extras) - experiments page: checkbox column + "Compare selected (n/8)" header action that navigates to /compare with the selected session ids (project-scoped when the selection stays within one project) Closes #105 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
#112) Snapshots were a passive record (dataset/code hashes + manifest). This adds an active reproduce path: - services/reproduce.py: load the snapshot manifest, verify current workspace files against the captured hashes (input drift), replay the captured .py scripts in a Modal sandbox (stage=None so the replay never writes into the original run's metric history), parse the metrics the replay emits, and diff final values vs the recorded run with a configurable tolerance — flagging drift/missing/new metrics. Report is also written to the volume as reproduce_report.json (best-effort). - POST /api/sessions/{id}/snapshot/reproduce with Pydantic request/response schemas (404 no snapshot, 409 manifest unreadable / no scripts). - Frontend: SnapshotReproduce component (button + status pill + metric diff table + input-drift warning) mounted in the experiment detail snapshot section; api.reproduceSnapshot + ReproduceReport types. - Tests: pure diff/verify/select logic + route-level tests with the sandbox call faked (drift, match, failed replay, 404). Closes #112 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
Adds a "Test" panel to live model cards on /models so the
train -> deploy -> validate loop closes without leaving the app:
- Backend proxy POST /api/models/{id}/predict forwards the batch to the
live Modal endpoint with the stored X-API-Key, so the browser never
holds the key and never fights Modal CORS. Upstream 4xx (e.g. key
drift 401) passes through; network errors and upstream 5xx surface as
502. Batches capped at 200 records.
- GET /api/models/{id}/predict-schema resolves the trained
feature_columns/target_column from the training dataset's metadata
(same lookup generate_serving_app uses, now extracted into
_resolve_training_metadata).
- Frontend Test panel renders a per-feature input form from the schema,
or accepts a small CSV upload (client-side parser, header row +
quoted-field support) when metadata is missing or batch testing is
wanted; predictions render inline in a table.
- 10 new backend tests covering proxy success/auth-forwarding, 404/409/
400 paths, upstream 401/500 mapping, schema resolution, and a
regression guard that generate_serving_app still embeds the resolved
feature columns.
Closes #109
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
…, trial budget, caps Give users say over the trainer agent before it runs (closes #104). Backend: - New TrainingConfig schema (optimization_metric, model_families, max_trials, max_wallclock_minutes, max_cost_usd) stored as project.training_config (JSON column + startup migration), editable via POST/PATCH /api/projects. - Agent runner injects a "## User training constraints" prompt block into any agent that can call start-training (orchestrator, trainer, chat), and clamps the training sandbox profile's per-call timeout to the wall-clock cap before sandbox_config flows into execute-code. - start-training skill now accepts optional optimization_metric and max_trials, and enforces the project's constraints at the skill boundary: disallowed framework, conflicting metric, or over-budget trial plan are rejected before the training window opens. Compliant calls echo the active constraints back to the agent. - trainer.yaml: user constraints override the default 30-50-trial strategy. Frontend: - ProjectSettingsModal grows a "Training Controls" section (metric input with suggestions, model-family chips, trial/wall-clock/cost caps); Sidebar saves sandbox_config + training_config together. No page.tsx changes. Projects with no training_config behave exactly as before (empty config renders no prompt block and skips all skill-boundary checks). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
…f metrics (closes #112) # Conflicts: # backend/schemas.py
- Move raw-file profiling into services/dataset_preview.py (thin routers) - Map duckdb.Error to 400; let server-side failures surface as 500 - Build preview query string with URLSearchParams (api.ts) - Add type="button" to the preview back button - prettier format frontend/src/components/ProjectDataModal.tsx
…pload (closes #103) # Conflicts: # backend/schemas.py # frontend/src/lib/api.ts
- Typed response models: SampleDatasetEntry, SampleProjectSummary, SampleExperimentSummary, ProjectFromSampleResponse in schemas.py - Move catalog scan + project-from-sample flow into services/samples.py (thin routers) - Move shared dataset path helpers into services/datasets.py as public API (no more cross-router underscore imports) - Offload sync file I/O (read_bytes, stat, is_file) via asyncio.to_thread - ruff format backend/routers/samples.py (+ reformatted service files) - prettier format frontend/src/app/experiments/page.tsx
…c, model families, trial budget, caps (closes #104) # Conflicts: # backend/schemas.py
# Conflicts: # backend/models.py # backend/routers/projects.py # backend/schemas.py # backend/services/agent/runner.py # frontend/src/components/ProjectSettingsModal.tsx # frontend/src/components/Sidebar.tsx # frontend/src/lib/api.ts # frontend/src/lib/types.ts
…p guardrail (closes #107)
…Greptile P1 on #87) Label inference (labels=None) iterated both sequences, so the sklearn and fallback paths received exhausted iterators and silently produced an all-zero matrix. Materialize to lists once up front; +2 regression tests (generator inputs, with and without explicit labels).
…ject zip endpoints (closes #79)
A throwing listener in `publish` ran inside connectSSE's try/catch before the UI switch, silently swallowing the event and killing all UI updates for that message. Wrap each listener invocation in its own try/catch.
|
Too many files changed for review. ( Bypass the limit by tagging |
lucastononro
added a commit
that referenced
this pull request
Jul 29, 2026
lucastononro
added a commit
that referenced
this pull request
Jul 29, 2026
5 tasks
lucastononro
added a commit
that referenced
this pull request
Jul 29, 2026
Conflicts/semantic reconciliation: - MetricsTab.tsx: #157 typed ChartTooltip props (dropped `any`); staging's #161 compare page imports it — kept the typed props AND the export. - SSE union extended with events other staging PRs added/consume: `budget_exceeded` (#165, new BudgetExceededSSEData) and the `notebook.kernel.state`/`notebook.structure.changed`/`notebook.cell.*` events dispatched by useNotebookSSE (#148's typed bus requires them). StructureChangedEvent moved to lib/notebook/types.ts (re-exported from useNotebookSSE) so lib/types.ts can reference it. - page.tsx budget_exceeded case adapted to #157's per-case `const data = event.data` block pattern. - useNotebookSSE.test.tsx: cell.completed payload gained exec_count: null to satisfy the now-typed CellCompletedEvent.
lucastononro
added a commit
that referenced
this pull request
Jul 29, 2026
Reconciliation with the #148 SSE-bus chain: - DROPPED useNotebookSSE.test.ts (stale — it tested the hook's own EventSource, which #148 replaced with the SSEStreamContext bus; #148's bus-based useNotebookSSE.test.tsx is the live suite). - api.test.ts add/add: combined both suites — #158's fetch-mocked fetchJSON/FormData helper tests AND staging's (#133) relative-URL builder tests — 15 tests total. - vitest.config.ts add/add: kept one config — staging's (#148) plus #158's globals + setupFiles (vitest.setup.ts / @testing-library/jest-dom). - package.json: union of devDeps (@testing-library/dom + jest-dom), single test script; package-lock.json: took staging's, regenerated via npm install. - ci.yml: #158's frontend-test job kept as-is (staging had none — #133 only wired the npm script); all 10 jobs intact, YAML valid.
Owner
Author
|
Merged into integration branch |
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
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.
Summary
HomePage.connectSSEanduseNotebookSSEeach opened an independentEventSourceto the identical/api/sessions/{id}/streamendpoint, doubling server fan-out and parsing every event twice.SSEStreamContext(src/lib/SSEStreamContext.tsx): a publish/subscribe bus exposing the single page-level stream.connectSSErepublishes every parsed message to it; the notebook now subscribes instead of opening a second connection.Changes
src/lib/SSEStreamContext.tsx—SSEStreamProvider+useSSEStream()(subscribe/publish pair).src/app/page.tsx— split the default export intoHomePageContent(unchanged logic) wrapped by a newHomePagethat mountsSSEStreamProvider;connectSSE'sonmessagenow callspublish(event)for every parsed message, in addition to its existing switch handling.src/lib/notebook/useNotebookSSE.ts— no longer opens its ownEventSource; subscribes to the shared stream viauseSSEStream()and keeps the exact samenotebook.*event filtering/dispatch logic.Test plan
Existing (must still work)
connectSSE's switch is untouched.metric/metrics_batchevents arrive.New
.ipynb) while an agent run is active — confirm cell outputs/kernel state/structure-changed events still update live, now sourced from the shared stream (verify in devtools Network tab that only oneEventSourceconnection to/api/sessions/{id}/streamis open, not two).useNotebookSSEcall cleans up via its returned unsubscribe on unmount/dep change).Closes #100
🤖 Generated with Claude Code
Greptile Summary
This PR eliminates duplicate
EventSourceconnections to/api/sessions/{id}/streamby introducing a lightweight pub/sub bus (SSEStreamContext).connectSSEinpage.tsxfeeds the bus viapublish, anduseNotebookSSEsubscribes to it instead of opening its own connection. A session-ID guard (eventSessionId !== sessionId) prevents cross-session event bleed during session switches, and new vitest tests cover fan-out, session isolation, and notebook filtering.SSEStreamContext.tsx: New stateless context holding stablesubscribe/publishcallbacks backed by aSet<SSEListener>ref \u2014 no re-renders triggered on consumers.page.tsx: Splits the default export intoSSEStreamProvider\u2192HomePageContent;connectSSE.onmessagecallspublish(sid, event)before its existing switch.useNotebookSSE.ts: Removes its ownEventSource; subscribes to the shared bus with asessionIdguard and stable handler/filter refs.Confidence Score: 4/5
Safe to merge after addressing listener error isolation in publish — as-is, a throwing subscriber silently aborts all chat/metrics/UI updates for that event.
The publish call sits inside connectSSE's try/catch block before the switch that drives every UI update. If any listener throws, the catch block swallows it and the switch never runs for that event — chat messages, metrics, and file-tree updates would be silently lost. The fix is small and localised, but it guards a critical code path.
frontend/src/lib/SSEStreamContext.tsx (publish forEach) and the call site in frontend/src/app/page.tsx where publish precedes the switch inside a swallowing catch block.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Backend as Backend SSE participant ES as EventSource (single) participant CSE as connectSSE.onmessage participant Bus as SSEStreamContext (pub/sub) participant NB as useNotebookSSE Backend->>ES: SSE event ES->>CSE: onmessage(e) CSE->>CSE: JSON.parse(e.data) CSE->>Bus: publish(sid, event) Bus->>NB: listener(sid, event) NB->>NB: sessionId guard check NB->>NB: "notebook.* filter + dispatch" CSE->>CSE: switch(event.type) — UI updates%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Backend as Backend SSE participant ES as EventSource (single) participant CSE as connectSSE.onmessage participant Bus as SSEStreamContext (pub/sub) participant NB as useNotebookSSE Backend->>ES: SSE event ES->>CSE: onmessage(e) CSE->>CSE: JSON.parse(e.data) CSE->>Bus: publish(sid, event) Bus->>NB: listener(sid, event) NB->>NB: sessionId guard check NB->>NB: "notebook.* filter + dispatch" CSE->>CSE: switch(event.type) — UI updatesReviews (2): Last reviewed commit: "fix(review): address Greptile findings o..." | Re-trigger Greptile