Multi User Support by samueljackson92 · Pull Request #387 · ukaea/toktagger

samueljackson92 · 2026-08-25T08:38:24Z

This PR implements a first pass at adding multi-user support to toktagger by:

Expanding the framework to support the concept of a "user" and annotations now have a user associated with them
Users have roles associated with them. Currently they are "admin", "editor" and "viewer"

Viewers can only view projects, they cannot edit or manage contributors
Editors can make and modify annotations, but cannot manage other accounts.
Admins can do everything.
Added guards to each API enpoint to prevent modifications without being an appropriately authenticated user
Added new UI pages to support this framework change
- Login page - users are now directed to the login page by default if not authenticated
- Admin control panel - accessible to admins only, can add, delete, and manage accounts
- User profile page - allows a user to see their login name and change their password.
Changed to using guicorn for python api for multi user support.
Updated user documentation
Updated setup development script
Updated test suite covering new functionality.

To test:
On default start-up, the default admin username and password will be printed in the command line terminal of the backend server.

check test suite passes successfully
check functionality of each type of user
check concurrent user annotation

- JWT-based auth using itsdangerous (stdlib-only, no system crypto dependency) with PBKDF2 password hashing; secret auto-persisted to ~/.cache/toktagger/ - First-run mode: when DB has no users, prints admin credentials to terminal and runs in passthrough mode (backward compatible with existing installs) - Concurrent annotation safety: update_annotations now scoped by created_by, so two users annotating the same shot no longer overwrite each other - Project membership (admin/annotator/viewer roles); non-admins see only their own projects; project admins manage membership via UI dialog - Per-user show_others_annotations toggle stored in project_members collection and enforced server-side in GET /annotations - New routes: POST /auth/token, GET /auth/me, CRUD /users, CRUD /projects/{id}/members - Frontend: Login page, Admin user management page, Project members dialog, AuthContext with token persistence, RequireAuth/RequireAdmin route guards, user info bar with logout on projects page, apiFetch wrapper on all API calls - Pre-existing fix: guard ModelRegistry/ray import under models_dependencies_installed() in routers/models.py and routers/meta.py https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW

https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW

Tests (45 total, all passing): - tests/api/auth/test_core.py — unit tests for hash_password, verify_password, create_access_token, decode_token (11 tests) - tests/api/auth/test_first_run.py — unit tests for ensure_admin_user (5 tests) - tests/api/auth/test_auth_router.py — /auth/token and /auth/me endpoints (8 tests) - tests/api/auth/test_users_router.py — /users and /projects/{id}/members CRUD (12 tests) - tests/api/auth/test_concurrent_annotations.py — concurrent annotation safety, identity enforcement, show_others filter, viewer access control (9 tests) Bugs fixed during test authoring: - MongoDBClient: file-path mode was ignoring the provided URL and always writing to the user cache dir, causing all tests to share a single DB (silent data corruption) - get_project_members: stored user_id ObjectId was not stringified before Pydantic validation, causing ValidationError on every member list call - GET /samples/{id}/annotations: had no project membership check, allowing any authenticated user to read any project's annotations (security gap) Test infrastructure: - tests/api/auth/conftest.py: self-contained fixtures using mongita disk client with per-test tmp_path isolation (no Docker required) - tests/conftest.py: guarded ray/ModelRegistry imports behind _models_available flag so auth tests can run without ray installed - pyproject.toml: added [tool.pytest.ini_options] asyncio_mode = "auto" https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW

- Add `require_project_viewer` dependency (any project member may read) - annotations.py: GET /annotations → viewer; PUT /annotations → annotator; DELETE /annotations and DELETE /samples/{id}/annotations → admin - samples.py: all 8 endpoints now guarded (reads→viewer, writes→annotator, deletes→admin); remove debug print from add_samples - data.py: POST /data guarded with viewer check - annotators.py: GET /annotator → viewer; POST /annotator/{type} → annotator - models.py: all 12 endpoints guarded alongside existing feature check - Add 19-test suite (test_endpoint_guards.py) covering non-member/viewer/ annotator/admin access across samples, project annotations, sample annotation delete, and data endpoints — all 64 auth tests pass https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW

…import enforcement - auth/core.py: add get_internal_token() for stable per-process server secret - auth/dependencies.py: accept internal token as synthetic admin (Ray worker callbacks authenticate as __internal__ without a DB round-trip) - main.py: pass API_TOKEN to Ray workers via runtime_env so sender.py can auth - core/sender.py: inject Authorization header when API_TOKEN env var is set - worker.py: prefix model prediction created_by with "model::" to prevent username collision (e.g. a user named "disruption_cnn" cannot corrupt model predictions via scoped annotation deletes) - routers/models.py: update delete_predictions filter to match "model::" prefix - routers/users.py: reject usernames starting with "model::" or "__" - routers/annotations.py: enforce created_by = current user for non-admin, non-internal bulk imports (prevents created_by spoofing) - modelPredictSample.tsx: update three created_by comparisons to use modelCreatedBy() helper that prepends "model::" prefix - test_model_auth.py: 8 tests covering all of the above (72/72 passing) https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW

The MONGO_URL default was accidentally set to "./toktagger_db" during Phase 1 auth work, placing the database inside the project directory instead of the intended ~/.cache/toktagger/ukaea/ location. Passing "default" triggers the existing user_cache_dir fallback in db.py. Also add toktagger_db/ to .gitignore to prevent accidental future commits of local database files. https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW

Five categories of collection/runtime failures, all pre-existing: 1. tests/db_definitions.py: unconditional `import ray` blocked 4 test files. Added a stub (no-op remote decorator) when ray is absent. Also changed MODEL_3 type from "disruption_cnn" to "mock_disruption_cnn" — the real type is only registered when ray is installed, so the Pydantic validator rejected it otherwise. 2. toktagger/api/models/base.py: @ray.remote on WorkerRegistry was outside the conditional import guard, causing NameError at import time. Added matching stub so the decorator is a no-op without ray. 3. tests/api/routers/test_models.py: direct `import ray` crashes collection. Changed to pytest.importorskip("ray") for clean skip. 4. tests/end_to_end/__init__.py: unconditional playwright import blocked all 4 e2e test files. Wrapped in try/except; each test file gets pytest.importorskip("playwright") for clean skip. 5. tests/conftest.py / test_annotator.py / test_data_loaders.py: - Docker not available: mongo_container fixture now skips when docker.from_env().ping() fails, turning 56 ERRORs into skips. - pooch (optional scipy dep) absent: test_annotator.py gets importorskip at module level. - FAIR-MAST external endpoint inaccessible: test now calls pytest.skip() on any network error rather than failing. Result: 91 passed, 159 skipped, 0 errors. https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW

Ruff lint (24 errors → 0): - Remove unused imports auto-fixed by ruff --fix - Rename unused local variables with _ prefix (original_loads, result, direction) to satisfy F841 - Add # noqa: E402 to imports after pytest.importorskip() in test_models.py (the skip call must precede imports) Ruff format (27 files → 0): - Auto-reformatted with ruff format ESLint (1 error → 0): - Remove unused useNavigate import from project_id/page.tsx Prettier (11 files → 0): - Auto-reformatted with prettier --write https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW

…ur assertions - tests/conftest.py: set app.state.auth_required=False in legacy api_client fixture so unauthenticated router tests pass (ensure_admin_user always returns True since the branch requires auth, but legacy tests have no token) - tests/conftest.py: update setup_model_samples created_by to "model::mock_disruption_cnn" to match the model:: namespace prefix introduced in commit 893ba2c - tests/api/crud/test_utils.py: update test_get_models_by_type and test_get_models_by_status to reflect MODEL_3 type change from "disruption_cnn" → "mock_disruption_cnn" (commit a06385f) - tests/api/routers/test_annotations.py: update annotation count (8 not 7) and skip created_by assertion since the server now overwrites it with the authenticated user's identity - tests/api/routers/test_models.py: update delete/stop tests to use "mock_disruption_cnn" throughout; fix test_model_delete_no_predictions to use "mock_timeseries_cnn" (type with no seeded predictions) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- toktagger/api/main.py: honour TOKTAGGER_AUTH_REQUIRED=false env var to bypass auth in the test server process (auth is always required in production, but E2E tests spin up a real server process that was timing out trying to get a 200 from /projects — now 401 is also accepted, and the test process disables auth via the env var so Playwright tests see the same UI they did before auth was added) - tests/conftest.py: set TOKTAGGER_AUTH_REQUIRED=false in run_server() so the E2E server starts in passthrough mode; also accept 401 from start_server health check so it doesn't wait 10 min before failing - .github/workflows/ci.yml: skip "Commit built files" push for fork PRs (github-actions[bot] cannot write to ukaea/toktagger from a fork PR) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Unit tests (~3 min) + E2E Playwright tests together now exceed 15 min since tests actually run rather than fast-failing on 401. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ired mode When TOKTAGGER_AUTH_REQUIRED=false, the server returns a synthetic admin from GET /auth/me with no token. The previous AuthContext short-circuited on mount if no token was in localStorage, so the frontend always redirected to /ui/login regardless of server auth state. Now AuthContext always calls /auth/me on mount; if the server returns a user (auth-not-required passthrough), RequireAuth lets the tests proceed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

The server now sets created_by = current_user.username on every annotation PUT. In the E2E environment (TOKTAGGER_AUTH_REQUIRED=false) the synthetic user is "admin", so saved annotations get created_by="admin", not "manual". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

testcontainers was imported in conftest.py but not listed in pyproject.toml, causing ModuleNotFoundError in both pytest CI jobs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…prefix The delete-predictions endpoint validates model_type against registered types and filters by 'model::<type>'. The fixture had created_by='disruption_cnn' (wrong type, wrong prefix); fix to 'model::mock_disruption_cnn'. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

The built frontend JS (from ukaea/dev) filters model annotations by bare model type string. The backend worker was adding a 'model::' prefix that the JS doesn't know about, so disabling the predict tool never cleared annotations. Use bare model.type in worker and delete-predictions filter. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- stop_model_training: use require_project_annotator (not admin) - crud/utils.py: remove unused _direction variable and pymongo import - test_auth_router: assert exactly 403 for deactivated-user login - test_model_auth: fix test_user_save_does_not_corrupt_model_prefixed_predictions to use a user actually named 'disruption_cnn' (was using 'alice') - test_users_router: add tests for non-admin update/delete-other-user (403) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

The session-scoped settings fixture was present on ukaea/dev but dropped during the branch rebase. ray_session (models_fixtures.py) depends on it for MODEL_STORAGE env var and setup_model_db uses config.settings.models.cache_dir. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

os.environ["MODEL_STORAGE"] is only set inside Ray worker processes (via runtime_env), not in the test process itself. Tests that read model file paths must use config.settings.models.cache_dir directly, which is patched to a temp directory by the session settings fixture. Similarly, DISABLE_LOCAL_MODEL_LOAD is not read by the router; the router checks config.settings.models.local_load_enabled instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

The built frontend JS identifies model predictions by created_by===model::${modelType}. Commit 1e959c5 incorrectly removed this prefix from the worker, so the Disable Tool toggle could never match and hide predictions. Restore the prefix in the worker and the delete_predictions filter, and update the fixture training data to match. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…nftest.py PR #269 (wk9874/use_mongita) replaced testcontainers with mongita on dev. When this branch merged, the conflict was kept incorrectly and testcontainers was re-added to pyproject.toml as a workaround. Align with dev: - Remove MongoDbContainer import and mongo_container fixture - db_client now depends on settings and uses MongoDBClient(mongo_url, ...) - api_client now depends on db_client and injects app.state directly - start_server now depends on settings (no MONGO_URL env var needed) - Remove testcontainers[mongodb] from dev dependencies Branch-specific additions are preserved: app.state.auth_required=False in api_client, TOKTAGGER_AUTH_REQUIRED=false in run_server, 600-step E2E wait. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

wk9874

Backend reviewed again, comments carried over from old PR

Will review UI and functionality test next

wk9874 · 2026-08-25T13:23:13Z

-    project = await utils.get_project(db_client, project_id)
-
-    if not project:
-        raise HTTPException(status_code=404, detail="Project not found with that ID.")


Why was this HTTPException removed (which was thrown when you tried to GET a project with a project ID which was not found in the DB)?

wk9874 · 2026-08-25T13:23:25Z

-    """Update a project's information.
-    -----------------------------
-    """
-    db_client: MongoDBClient = request.app.state.db_client


Why remove this? We do this in most endpoints, should probably do it in all for consistency

Should really have db_client: MongoDBClient = request.app.state.db_client everywhere to aid with type hints

wk9874 · 2026-08-25T13:25:33Z

Note that I fixed a series of issues with the tests which I would have otherwise raised as PR comments here: #386

That should ideally be reviewed and merged into this branch, before this goes into dev

wk9874 · 2026-08-25T13:42:03Z

MF: Pressing 'Clear' on the UI with 'show others' annotations' on also removes their annotations from my UI. If I press save, it saves 0 annotations (correctly), but then if I reload the page it shows the others' annotations again

SJ: I have changed this so that is clears whatever is visible. If only your annotations are visible then they are cleared. If all annotations are visible, then all annotations are cleared. This makes sense to me?

So we're saying one user can clear another user's annotations? I think that makes sense, since it should be collaborative... But maybe worth documenting?

Although this behaviour is now not consistent with deleting an annotation.. If one user deletes a specific annotation from another user and saves it, the other users annotation will reappear on page refresh. Should make this work similar to Clear

wk9874 · 2026-08-25T13:48:57Z

For a project viewer, Edit mode, Save, annotators etc are correctly greyed out. However when changing sample, they briefly re-render as being available before being greyed out again. Can this be fixed?

toktagger_change_sample.mp4

Also should Viewers be able to train & predict with models? Currently they can, but I'm not sure whether they should or not. Note that predictions will go straight into the database. Maybe they shouldn't be able to....

wk9874 · 2026-08-25T13:51:41Z

We now do an awful lot of API calls when loading a sample, including three (!) separate calls to project members endpoint. Are these all necessary?

INFO:     127.0.0.1:52138 - "GET /ui/projects/6a8d9967c41355ca9c40924e/samples/6a8d9967c41355ca9c409252?sortColumn=shot_id&sortDirection=ascending HTTP/1.1" 200 OK
INFO:     127.0.0.1:52138 - "GET /auth/me HTTP/1.1" 200 OK
INFO:     127.0.0.1:52138 - "GET /health HTTP/1.1" 200 OK
INFO:     127.0.0.1:52138 - "GET /openapi.json HTTP/1.1" 200 OK
INFO:     127.0.0.1:52138 - "GET /projects/6a8d9967c41355ca9c40924e HTTP/1.1" 200 OK
INFO:     127.0.0.1:52150 - "GET /projects/6a8d9967c41355ca9c40924e/samples/6a8d9967c41355ca9c409252 HTTP/1.1" 200 OK
INFO:     127.0.0.1:52138 - "GET /projects/6a8d9967c41355ca9c40924e/samples/6a8d9967c41355ca9c409252/annotations HTTP/1.1" 200 OK
INFO:     127.0.0.1:52138 - "POST /projects/6a8d9967c41355ca9c40924e/samples/6a8d9967c41355ca9c409252/data HTTP/1.1" 200 OK
INFO:     127.0.0.1:52138 - "GET /users/me/memberships HTTP/1.1" 200 OK
INFO:     127.0.0.1:52150 - "GET /projects/6a8d9967c41355ca9c40924e/members HTTP/1.1" 200 OK
INFO:     127.0.0.1:52138 - "GET /projects/6a8d9967c41355ca9c40924e/members HTTP/1.1" 200 OK
INFO:     127.0.0.1:52138 - "GET /projects/6a8d9967c41355ca9c40924e/members HTTP/1.1" 200 OK

Should it be calling /auth/me every time? Shouldn't it be caching the token?

I also dont think it should be calling /health every time, it should call this once when the context is created and persist it for the rest of the session. Although I dont know if this has gone wrong in this PR or a separate one

A single 45m job timeout hides which step got slow. Reserve 20m for the playwright install, whose duration swings with GitHub's CDN throughput, and 15m for each pytest run, so a regression in test time is visible between commits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

docker-compose.yml read ${WORKERS} while configuration.md and user_management.md both document SERVER_WORKERS, so the documented variable had no effect. Name it SERVER_WORKERS everywhere, matching the Settings field. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The save path dropped any edit to an "annotators::" or "model::" annotation and left the stored row untouched, so correcting a prediction and pressing Save lost the correction - the opposite of what the tool is for. The edit is now applied in place, keeping the synthetic author so provenance is not rewritten. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Annotations edited in place were never added to the response, so the endpoint reported only the subset that went through the delete-then-reinsert path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Every endpoint now declares the type its response_model already promises. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

must_change_password was taken from the request body, so a client could create an account that never has to replace the password the admin chose for it. The flag is now set by the server; the e2e helper clears it afterwards for accounts that need to be usable straight away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

wk9874 · 2026-08-25T15:38:00Z

scripts/create_mock_data.py doesn't work:

python scripts/create_mock_data.py 
Password: 
Traceback (most recent call last):
  File "/home/wk9874/Documents/toktagger/scripts/create_mock_data.py", line 133, in <module>
    main()
  File "/home/wk9874/Documents/toktagger/scripts/create_mock_data.py", line 109, in main
    create_local_samples(
  File "/home/wk9874/Documents/toktagger/scripts/setup.py", line 164, in create_local_samples
    r.raise_for_status()
  File "/home/wk9874/Documents/toktagger/.venv/lib/python3.12/site-packages/requests/models.py", line 1167, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 422 Client Error: Unprocessable Entity for url: http://localhost:8002/projects/6a8db692c41355ca9c40926d/samples

Was inconsistent per-endpoint; do it everywhere for the IDE/type-checker support it gives call sites, matching a PR review comment.

The batch save only replaces the caller's own annotations, so removing someone else's (or a model's) locally used to be undone by the next save -- it reappeared on reload. Explicitly DELETE removed annotations the caller doesn't own, the same way Clear already does.

Several docstrings and inline comments ran to multi-paragraph explanations; trim them to a sentence or two of the non-obvious "why".

wk9874 · 2026-08-25T15:43:12Z

A project can have all of their admins relegated to annotators or below, but I guess we probably don't mind too much about this? Since top level admins can access any project, and they can just rectify it if needed

wk9874 · 2026-08-25T15:44:35Z

Project admins can reduce the privileges of global admins in their project, but this will have no effect (since global admins can see everything anyway). Again we probably dont care about this too much?

Spectrum renders a breadcrumb trail's last item as the current, unclickable page. A trail with only "Projects" in it (the case whenever the project itself fails to load) silently lost its link, trapping the user on the error screen. Add a terminal "Access Denied"/"Error" crumb so "Projects" is no longer last.

The earlier fix for persisting a delete of another author's annotation also changed which annotations a save marks validated, based on created_by matching the caller. That missed a brand-new annotation (hand-drawn, or an annotator's just-run suggestion) whose created_by is a synthetic prefix, not the caller's username, so its first save silently stopped validating it. Key off whether the annotation has a server _id yet instead.

The toolbar/nav bar remount on every sample change (the stale-render guard in page.tsx), and useProjectRole's isAdmin/canAnnotate default open until its membership fetch resolves. Called locally in each of those components, that meant a fresh default-open window on every single navigation, briefly re-enabling Save/Edit for a viewer before the re-fetch caught up. Move the role lookup into SampleContext, which sits above the remounting subtree and survives sample navigation, so the membership check only runs once per project and the disabled state never flickers.

These write predictions/models into the project, which the backend already restricts to require_project_annotator - the frontend just wasn't matching that, so a viewer saw live-looking controls that would 403 on click.

samueljackson92 · 2026-08-26T12:41:37Z

MF: Pressing 'Clear' on the UI with 'show others' annotations' on also removes their annotations from my UI. If I press save, it saves 0 annotations (correctly), but then if I reload the page it shows the others' annotations again

SJ: I have changed this so that is clears whatever is visible. If only your annotations are visible then they are cleared. If all annotations are visible, then all annotations are cleared. This makes sense to me?

So we're saying one user can clear another user's annotations? I think that makes sense, since it should be collaborative... But maybe worth documenting?

Although this behaviour is now not consistent with deleting an annotation.. If one user deletes a specific annotation from another user and saves it, the other users annotation will reappear on page refresh. Should make this work similar to Clear

Yes. This is what I am saying. I think either you can only edit/delete your own annotations or you can edit everyone's.

wk9874

UI reviewed

Side note - this PR is huge! While this is probably somewhat inevitable with a change as big as adding authentication, it makes reviewing pretty difficult & time consuming. In the future we should try to create feature/... branches, and then have smaller branches feeding into those which can be reviewed independently :)

Also, there are a few changes in here about handling page layouts, resizing logic etc. I personally think this all needs a bit more thought, and should go into a separate PR later down the line as they arent required for adding Auth

wk9874 · 2026-08-25T16:16:10Z

-            created_by: "manual",
+            // Authored by whoever selected the label; the server stamps the same
+            // username on save.
+            created_by: user?.username ?? "manual",


I'm not sure about this, will come back to it...

(basically my thinking was - if the server overwrites the created_by field anyway with the user's ID, do we need this? I guess it doesnt hurt...)

wk9874 · 2026-08-25T16:16:57Z

  PROFILE_2D_THRESHOLD = "profile_2d_threshold",
 }
+
+// Mirrors the "annotators::<type>" prefix the backend stamps on annotator


Don't need all these comments

wk9874 · 2026-08-26T09:08:13Z

+        )}
+      </div>
+      <div className="flex items-center gap-2 flex-none">
+        <span className="text-sm text-gray-600 dark:text-gray-300">


Could use a react spectrum container here instead for consistency, eg a Text?

wk9874 · 2026-08-26T09:17:56Z

@@ -0,0 +1,51 @@
+"use client";


This file doesnt fit the naming convention, should be topBar.tsx

Also I note that this has gone into a new folder called layout with only this thing in. I wonder if either this should somewhere else, like tools, or if other layout-y files (like nav.tsx, toolbar.tsx) should be moved into here?

wk9874 · 2026-08-26T09:35:04Z

-        <Text>Clear</Text>
-      </ActionButton>
+      <TooltipTrigger delay={1000} placement="bottom">
+        <ActionButton


Since this now has the power to delete all users' annotations at once, maybe we should add a confirmation popup?

Probably only required in the case where Show Others' Annotations is True

wk9874 · 2026-08-26T14:57:29Z

+          <span style={{ fontSize: "15pt" }}>403 - Forbidden</span>
+        </Header>
+        <div style={{ fontSize: "48px", marginBottom: "16px" }}>🔒</div>
+        <p style={{ color: "#666", maxWidth: "500px", textAlign: "center" }}>


Text object instead?

wk9874 · 2026-08-26T14:58:36Z

+        <Header>
+          <span style={{ fontSize: "15pt" }}>403 - Forbidden</span>
+        </Header>
+        <div style={{ fontSize: "48px", marginBottom: "16px" }}>🔒</div>


Use a react spectrum icon for a lock instead to keep consistent theme

wk9874 · 2026-08-26T14:59:27Z

 };

+// ---------------------------------------------------------------------------
+// Auth / User types


wk9874 · 2026-08-26T15:00:40Z

-      MODELS_CACHE_DIR: "/app/data/models"
-      SERVER_HOST: api_app
-      SERVER_PORT: 8002
+      MONGO_URL: "mongodb://${MONGO_USERNAME}:${MONGO_PASSWORD}@mongo:27017"


dont think these should have changed names...?

wk9874 · 2026-08-26T15:01:01Z

-      SERVER_PORT: 8002
-      SERVER_RELOAD: "false"
-      CUSTOM_SCRIPT: ${CUSTOM_SCRIPT}
+      MONGO_URL: "mongodb://${MONGO_USERNAME}:${MONGO_PASSWORD}@mongo:27017"


same here, shouldn't have changed names

also API_URL no longer required

abdullah-ukaea

Two general observations I noticed in the code when I glanced at the git diff. @samueljackson92

I noticed quite a few multi-line comments that seem to explain code whose intent is already reasonably clear for example, in NavAdapterContext.tsx. It has been pointed out in other PRs e.g. #349 (review)

Our CLAUDE.md asks us to default to no comments, use concise single-line comments for non-obvious “why” explanations, and reserve multi-line comments for rare cases.

Could you please ask Claude to review the UI comments and remove or condense them wherever possible, keeping multi-line comments only where a single line genuinely cannot provide enough context or where a docstring is appropriate?

It may also be worth reinforcing this guidance in CLAUDE.md for future changes

I haven't reviewed the implementation in detail, but I wanted to flag a concern about the overall size of the change. I appreciate that multi-user support is non-trivial, but +11,000/−3,323 lines across 158 files seems unusually large for adding multi user functionality.

abdullah-ukaea · 2026-08-27T14:19:36Z

+// Deletes what the user removed but a save cannot: the batch save replaces only the
+// caller's own annotations, so another author's stays until deleted outright. Shared
+// with the video adapter, which works from the same sample-wide annotation set.


reduce to 1 line?

abdullah-ukaea · 2026-08-27T14:19:50Z

+      // Clearing what the user can see means clearing other users' annotations and
+      // model predictions too. A save cannot do that - its replace step is scoped to
+      // the caller's own created_by - so they are deleted here explicitly, and the
+      // local view is only emptied once that succeeds.


reduce to 1 line?

abdullah-ukaea · 2026-08-27T14:19:57Z

+      // "Show others" is off, so the user can only see their own annotations and
+      // only those are cleared. They are removed from the local view alone; the save
+      // that follows is what deletes them server-side. "manual" is the placeholder
+      // used until the auth context resolves, so it belongs to whoever is drawing.


reduce to 1 line?

claude and others added 30 commits July 7, 2026 14:09

Ignore TypeScript incremental build cache files

fe2ab22

https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW

Add update user management & styling

0847228

Remove db

7c8bdcc

Add dep

46049bc

Increase Pytest CI timeout from 15 to 30 minutes

c04aa53

Unit tests (~3 min) + E2E Playwright tests together now exceed 15 min since tests actually run rather than fast-failing on 401. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Ruff linting

7ff5189

Fix CI: add testcontainers to dev deps, remove unused import

3b8acf9

testcontainers was imported in conftest.py but not listed in pyproject.toml, causing ModuleNotFoundError in both pytest CI jobs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Fix issues from review after merge

1fedd18

Update to fix UI issues

ee19ad0

Update docs, fix warnings

0c8ca18

Update to address PR comments

587089d

fix first run password

235a71b

wk9874 requested changes Aug 25, 2026

View reviewed changes

wk9874 closed this Aug 25, 2026

wk9874 reopened this Aug 25, 2026

This comment was marked as resolved.

Sign in to view

samueljackson92 and others added 8 commits August 25, 2026 15:41

Resolve issues from CI

8af078a

docs: drop unused config import from the docker model example

679857d

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Return every updated annotation ID from the batch save

61d1eea

Annotations edited in place were never added to the response, so the endpoint reported only the subset that went through the delete-then-reinsert path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Annotate return types on the users router

35d0d19

Every endpoint now declares the type its response_model already promises. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

samueljackson92 added 3 commits August 25, 2026 16:38

Type-hint db_client as MongoDBClient across all routers

7dcbbab

Was inconsistent per-endpoint; do it everywhere for the IDE/type-checker support it gives call sites, matching a PR review comment.

Condense overlong comments flagged in review

37b5a12

Several docstrings and inline comments ran to multi-paragraph explanations; trim them to a sentence or two of the non-obvious "why".

samueljackson92 added 4 commits August 25, 2026 16:50

Disable Train/Load/Predict ML model buttons for project viewers

201c918

These write predictions/models into the project, which the backend already restricts to require_project_annotator - the frontend just wasn't matching that, so a viewer saw live-looking controls that would 403 on click.

Rebuild ui

6028720

wk9874 requested changes Aug 26, 2026

View reviewed changes

abdullah-ukaea requested changes Aug 27, 2026

View reviewed changes

Uh oh!

Conversation

samueljackson92 commented Aug 25, 2026

Uh oh!

wk9874 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wk9874 commented Aug 25, 2026

Uh oh!

wk9874 commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wk9874 commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wk9874 commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

wk9874 commented Aug 25, 2026

Uh oh!

wk9874 commented Aug 25, 2026

Uh oh!

wk9874 commented Aug 25, 2026

Uh oh!

samueljackson92 commented Aug 26, 2026

Uh oh!

wk9874 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abdullah-ukaea left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

wk9874 commented Aug 25, 2026 •

edited

Loading

wk9874 commented Aug 25, 2026 •

edited

Loading

wk9874 commented Aug 25, 2026 •

edited

Loading