Add the configurable Bublik AI assistant backend - #348
Conversation
459c3d2 to
1202c2b
Compare
3649d9d to
1937329
Compare
| return ChatThread.objects.filter(user=user) | ||
|
|
||
| def _get_owned_or_none(self, pk, user): | ||
| thread = ChatThread.objects.filter(pk=pk).first() |
There was a problem hiding this comment.
pk isn't validated as a UUID here — .filter(pk=pk) will raise django.core.exceptions.ValidationError on malformed input, which becomes an unhandled 500 (this bypasses DRF's get_object_or_404, which normally catches it). Same issue is already handled in access.py::_thread_owner_id — suggest mirroring that try/except (ValidationError, ValueError) here.
| # Create the thread row atomically before persisting the user turn. The DB | ||
| # remains authoritative for ownership and the complete visible transcript. | ||
| # If the thread already belongs to a different user, reject. | ||
| thread, _created = await sync_to_async(ChatThread.objects.get_or_create)( |
There was a problem hiding this comment.
thread_id isn't validated before get_or_create(pk=thread_id, ...). A malformed UUID raises ValidationError, and this Starlette app has no catch-all exception handler, so it surfaces as a raw 500 instead of 422/404.
| """ | ||
| client = _client() | ||
| thread_key = _thread_key(thread_id) | ||
| existing_run_id = await client.get(thread_key) |
There was a problem hiding this comment.
Minor: check-then-act race — the get here and the pipe.execute() below aren't atomic, so two near-simultaneous register_run calls for the same thread could both pass the ConcurrentRunError check. pipeline(transaction=True) doesn't fix this since it can't branch on a value read in the same atomic block. Suggest a single Lua script (EVAL) instead. Narrow window, not blocking.
| # 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', | ||
| # 'LOCATION': 'redis://127.0.0.1:11211', | ||
| # 'TIMEOUT': 60 * 20, | ||
| 'BACKEND': 'django_redis.cache.RedisCache', |
There was a problem hiding this comment.
Pointing 'default' at real Redis silently activates UpdateCacheMiddleware/FetchFromCacheMiddleware — they cache whole GET responses app-wide, for every existing view, none written with that in mind.
Why was this changed? Nothing in the commit message explains it. If it's for discovery.py's cache.get/cache.set calls, suggest giving that its own dedicated alias instead, and reverting 'default' back to DummyCache.
| ) | ||
| extra_kwargs: ClassVar[dict] = { | ||
| 'title': {'required': False}, | ||
| 'messages': {'required': False}, |
There was a problem hiding this comment.
messages is required: False here but not read_only, so ChatThreadViewSet.partial_update (which uses this serializer directly with raw request.data) actually lets a client overwrite the stored
transcript via PATCH .../chat-threads/<id>/ {"messages": [...]}.
This directly contradicts the ViewSet's own docstring: "clients never write conversation history directly." Suggest 'messages': {'read_only': True} here.
There was a problem hiding this comment.
This deletes leftover no-project configs on the assumption that each now has a per-project copy from init_project_configs. That's true for PER_CONF/REFERENCES/META, but not AI — it's excluded from init_project_configs on purpose, since it's not project-specific. So this line deletes the only copy of AI. Suggest excluding it here too: .exclude(type=ConfigTypes.GLOBAL, name=GlobalConfigs.AI.name).
|
|
||
|
|
||
| def _save_messages(thread_id: str, messages: list[dict]) -> None: | ||
| thread = ChatThread.objects.get(pk=thread_id) |
There was a problem hiding this comment.
Unguarded ChatThread.objects.get() — raises if the thread was deleted mid-run. compaction.py::_write_state already guards this same race with .filter(...).first() + early return; suggest the same here.
Decouple tool implementations from FastMCP registration so the chat agent can reuse the same validated operations without an MCP network hop. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Discriminate structured log blocks, clarify leaf-result navigation, and include report configurations so agents receive unambiguous responses they can act on reliably. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Add the provider, Redis, rendering, storage, ASGI, and test packages required by the chat backend, and allow unittest assertions in its Django tests. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Define providers and model discovery before runtime behavior, resolve secrets only from qualified environment or Django settings references, and keep process-wide AI configuration on the default project. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Store complete conversation transcripts in private per-user threads so server-managed runs can survive page reloads without exposing one user's history to another. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Render agent-created artifacts into bounded S3 objects and enforce thread ownership on downloads so generated files remain useful without becoming publicly accessible. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Build cached agents from validated provider settings and attach local or remote MCP tools so deployments can select models without changing application code. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Run agents in tracked background tasks, buffer live events in Redis, and persist complete transcripts on the server so conversations finish reliably across browser disconnects. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Persist each successful run's token occupancy and emit it to the client so users can understand model limits and later compaction decisions. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Summarize older turns only when a model approaches its context limit while retaining the full stored transcript, recent messages, and reusable summary state. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Publish the deployment-controlled chat flag through the server feature response so the frontend only presents chat when operators have enabled it. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Run systemd installations with an ASGI worker, disable proxy buffering for chat streams, and provide Redis and SeaweedFS defaults required by durable runs and generated files. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
| # Sheet titles are limited to 31 chars and a few forbidden characters. | ||
| sheet.title = re.sub(r'[\\/*?:\[\]]', '_', title)[:31] or 'Sheet1' | ||
| for row in rows: | ||
| sheet.append(row) |
There was a problem hiding this comment.
Formula injection: =-prefixed values are auto-detected by openpyxl and saved as live formulas. Please sanitize (prefix with ') before appending; worth covering +/-/@ too as defense-in-depth.
| def _render_csv(rows: list[list]) -> bytes: | ||
| buffer = io.StringIO() | ||
| writer = csv.writer(buffer) | ||
| writer.writerows(rows) |
There was a problem hiding this comment.
Same formula-injection issue as _render_xlsx above — please sanitize cell values here too before writing.
| basename='tests_comments', | ||
| ) | ||
| api_v2_router.register(r'config', api_v2.ConfigViewSet, 'config') | ||
| api_v2_router.register(r'chat/threads', api_v2.ChatThreadViewSet, 'chat-threads') |
There was a problem hiding this comment.
chat/threads is registered unconditionally, unlike analytics below (settings.ANALYTICS_ENABLED). Could this follow the same pattern?
|
|
||
| application = Starlette( | ||
| routes=[ | ||
| *build_chat_routes(), |
There was a problem hiding this comment.
Why not gate this behind settings.CHAT_ENABLED and skip calling build_chat_routes() when it's off, so chat endpoints simply don't exist instead of just being hidden in the UI?
| if config.compaction.enabled and context_limit: | ||
| # Same config/credential path as build_agent above, which already | ||
| # succeeded -- so this cannot introduce a new user-facing failure. | ||
| summarizer_model = await sync_to_async(build_model)(provider, model) |
There was a problem hiding this comment.
Why not wrap the setup code from here through the spawn_run call (line 176) in try/except that calls run_store.finish_run(run_id, 'error') on failure — same as the persist_messages block above? Right now any exception in this range leaves the thread stuck in running for the full TTL.
| run_key = _run_key(run_id) | ||
| async with client.pipeline(transaction=True) as pipe: | ||
| pipe.xadd(run_key, {DATA_FIELD: sse}) | ||
| pipe.expire(run_key, _EVENT_TTL) |
There was a problem hiding this comment.
Why not also refresh chat:thread:{id}:run's TTL here (same pipeline)? It's currently only set in register_run/finish_run, so a run longer than 1h loses its thread pointer mid-run.
| RunErrorEvent(message='Run cancelled.', code='cancelled') | ||
| ) | ||
| await run_store.append_event(run_id, error) | ||
| await run_store.finish_run(run_id, status) |
There was a problem hiding this comment.
On cancel/error, persist_messages never runs (only fires in on_complete, which pydantic-ai only calls on clean success), so any partial assistant output — and any already-executed side effects like generate_file's S3 upload — is lost from the thread's history with no trace. Is there a way to reconstruct and persist at least the buffered AG-UI events (already captured via append_event) on this path, since result.new_messages() isn't available here?
2 participants
Summary
Add an authenticated AI chat backend that can investigate Bublik data through
the existing MCP tool implementations, persist user-owned conversations, run
generation after browser disconnects, compact long contexts, and generate
downloadable files in S3-compatible storage.
Related
Changes
${env:AI_*}and${settings:AI_*}secret references.streaming, and transcript conversion.
Configuration
AI configuration is a global config named
aiand is restricted to No Project(Default). Providers may define explicit models or use model discovery. The
default compaction threshold is 80% of model context while retaining the eight
most recent messages.
Generated-file settings:
S3_ENDPOINT_URLS3_PUBLIC_ENDPOINT_URLS3_ACCESS_KEYS3_SECRET_KEYS3_BUCKETS3_REGIONS3_PRESIGN_EXPIRYCHAT_FILE_MAX_SIZE