Add topic summary metadata and actions - #84
Conversation
a5afcc3 to
d5f6d18
Compare
phantomii
left a comment
There was a problem hiding this comment.
Changes requested:
-
set_summarymust not be a public REST action. Topic summaries are written only by the server-side agent, so remove this route and OpenAPI contract while retaining an internal helper for the agent. -
set_summary_promptmust be limited to stream owners and administrators. The current allowed-role set also permits moderators; remove that role and update the corresponding tests and API documentation.
d5f6d18 to
6a21b41
Compare
|
Both review points are now reflected in the contract. Topic-summary writes no longer have a public route or OpenAPI operation; the server-side helper remains the only write path, including boundary validation, stale-job protection, and The updated commit is |
|
Please add the following in a new commit before this PR is approved. Implement server-side topic summarization on the existing
The shared summary remains readable by all users included in the stream; only the server-side worker writes it. |
|
One protocol constraint for the new endpoint registry and worker: every configured LLM endpoint must use the OpenAI-compatible API. Store the base URL and model identifier, validate the compatible request/response contract, and invoke the chat-completions-compatible interface. Do not introduce provider-specific client branches in this first implementation. |
|
Add one endpoint capability field: |
|
Refine the vision scheduling rule:
|
|
Add standard generation settings to each endpoint configuration and apply them to every OpenAI-compatible request. At minimum persist and validate |
|
Additional summarization requirements, consolidated:
|
|
One additional requirement: topic summarization must also have a global enable/disable flag. The worker may process a topic only when the global flag and that project's feature setting are both enabled. |
|
The additional functionality is now implemented in the separate commit The existing Messenger worker now claims one eligible topic and at most 100 new messages per step, snapshots the message boundary, effective prompt, and per-summary reasoning choice, then commits that claim before calling the model. Provider I/O therefore runs outside database transactions, and successful results still enter through the existing internal summary helper. Topic and endpoint leases, retry timing, attempts, and failure codes are persisted and bounded. The control plane is database-backed: both the global flag and the current project's flag must be enabled, while endpoint selection remains global. Endpoint CRUD/reprioritization is protected by Every endpoint uses the OpenAI-compatible chat-completions contract with a base URL and model. Selection is deterministic by priority and UUID, and retryable failures release the lease and move to the next enabled endpoint. The persisted non-secret configuration includes The vision scheduling cases are covered end to end: a topic waits when its new image has an enabled but busy vision endpoint, without touching a free text endpoint; when no vision endpoint is enabled, the same kind of update is summarized text-only. The same test proves a real 503 failover, explicit lease release, capability-aware reasoning, images only in user content, and a text-only system prompt. Local validation is green with 518 unit tests, 117 filtered integration tests (the same six tracked baseline exclusions), the functional suite, Ruff, and mypy. All 15 GitHub checks on |
phantomii
left a comment
There was a problem hiding this comment.
The two earlier requests are fixed. A full lifecycle and concurrency review found three remaining correctness problems, all reproduced against the current head. The existing suite is green (518 unit tests, 7 focused integration tests, 6 migration cleanup tests, Ruff, and mypy), but it does not exercise these cases. Please address the inline findings before merge.
bbbe20a to
066087b
Compare
|
Also added the requested per-topic summary switch in 066087b. Owners and administrators can send summary_enabled through the existing summary-configuration action; false cancels pending work and prevents new claims while preserving the current summary, and true allows refreshes again. A result already in flight is discarded if the topic is disabled before completion. |
Summary
summary_has_new_messagesin user topic projections and keep summary writes on the server-side internal helper; no publicset_summaryaction existsCommits
6a21b416— topic summary metadata and the reviewed server-only write contractbbbe20a5— server-side worker, endpoint registry, feature gates, capabilities, retries, and administrative APIValidation
pytest -q workspace/tests/unit— 518 passedpytest -q workspace/tests/integration -k "not test_verified_provider_identity_replaces_account_scoped_duplicates and not test_external_projection_move_is_request_atomic_after_each_phase and not test_external_visibility_uses_the_canonical_stream_per_logical_chat"— 117 passed, 6 deselectedpytest -q workspace/tests/functional— 1 passedmypy -p workspace— passed (87 source files)ruff check .— passedThe six excluded integration cases reproduce unchanged on the current default branch and are already tracked in #30, #65, and #79.
Migration order
Migration
0129adds the worker settings, global endpoint registry, encrypted credential envelope, explicit claims, and bounded job state; it depends on this PR's0128topic-summary metadata migration.