feat(summarizer): point summary endpoint at Studio (retire context-memory)#622
Merged
Conversation
…mory) Default the workflow-summary base URL to https://app.avaprotocol.org, where Studio now hosts /api/summarize with the identical request/response contract. The Go request/ response structs are unchanged — connecting is a config repoint (NotificationsSummary.APIEndpoint in avs-infra) + rotated APIKey.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR repoints the task engine’s AI workflow summarization client from the deprecated context-memory service to Studio by updating the default base URL used for the /api/summarize call, while keeping the existing request/response structs and call pattern unchanged.
Changes:
- Update
ContextAPIURLdefault fromhttps://context-api.avaprotocol.orgtohttps://app.avaprotocol.org. - Expand the inline constant documentation to describe the Studio migration and configuration override path.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+19
to
+23
| // ContextAPIURL is the default base URL for the workflow-summary endpoint. | ||
| // Migrated from the deprecated context-memory service to Studio, which now hosts | ||
| // /api/summarize with the identical request/response contract. The "/api/summarize" | ||
| // path is appended by the client. Override per-environment via | ||
| // NotificationsSummary.APIEndpoint (avs-infra config) and rotate APIKey. |
Addresses Copilot review on #622 — document that NotificationsSummary.APIEndpoint must be an origin only (client appends /api/summarize); a path or trailing slash would double the URL. Note the legacy constant name.
…nfig Remove the ContextAPIURL fallback constant so the summarizer endpoint is never silently defaulted to a baked-in origin. NewContextMemorySummarizer- FromAggregatorConfig now returns (Summarizer, error): (nil, nil) when notifications.summary is disabled, but an error when it is ENABLED yet misconfigured (unsupported provider, or empty api_endpoint/api_key). The engine treats that error as fatal at startup, so a broken summarizer config surfaces loudly instead of degrading to the deterministic path. Tests use a clearly-named test-only defaultSummarizerURL fallback.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
core/taskengine/summarizer_context_memory.go:36
NewContextMemorySummarizer's docstring says callers must pass a bare origin with no trailing slash and no/api/summarize, but the constructor does not normalize. Since the request path is built via string concatenation (c.baseURL + "/api/summarize"), a trailing slash or already-included path will produce malformed/doubled URLs. Normalizing in the constructor makes all call sites (including tests) more robust.
func NewContextMemorySummarizer(baseURL, authToken string) Summarizer {
return &ContextMemorySummarizer{
baseURL: baseURL,
authToken: authToken,
httpClient: &http.Client{Timeout: 30 * time.Second},
}
Comment on lines
85
to
+87
| // Override context_api_endpoint to use mock server | ||
| mockConfig.MacroSecrets["context_api_endpoint"] = contextMemoryServer.URL | ||
| summarizer := NewContextMemorySummarizerFromAggregatorConfig(&mockConfig) | ||
| summarizer, err := NewContextMemorySummarizerFromAggregatorConfig(&mockConfig) |
Comment on lines
+127
to
+130
| baseURL := strings.TrimSpace(c.NotificationsSummary.APIEndpoint) | ||
| if baseURL == "" { | ||
| baseURL = ContextAPIURL | ||
| return nil, fmt.Errorf("notifications.summary.enabled is true but api_endpoint is empty (set SUMMARIZER_API_URL to the Studio origin, e.g. https://app.avaprotocol.org)") | ||
| } |
Comment on lines
+112
to
+120
| // NewContextMemorySummarizerFromAggregatorConfig builds the workflow summarizer from the | ||
| // aggregator's notifications.summary config (api_endpoint + api_key). | ||
| // | ||
| // Returns (nil, nil) when summarization is disabled — a valid "off" mode in which callers | ||
| // use the deterministic summarizer. Returns (nil, error) when summarization is ENABLED but | ||
| // misconfigured (unsupported provider, or empty endpoint/key) so the daemon fails fast at | ||
| // startup instead of silently degrading. There is no hardcoded endpoint default: the origin | ||
| // must come from config (avs-infra: ${SUMMARIZER_API_URL} / ${SUMMARIZER_API_KEY}). | ||
| func NewContextMemorySummarizerFromAggregatorConfig(c *config.Config) (Summarizer, error) { |
Comment on lines
+342
to
+347
| contextMemorySummarizer, err := NewContextMemorySummarizerFromAggregatorConfig(config) | ||
| if err != nil { | ||
| // notifications.summary is enabled but misconfigured — refuse to boot rather than | ||
| // silently fall back, so a broken summarizer config surfaces loudly at startup. | ||
| logger.Fatal("Invalid notifications.summary configuration — refusing to start", "error", err) | ||
| } |
chrisli30
pushed a commit
that referenced
this pull request
Jun 23, 2026
…eanup, error-path tests Follow-up to #622/#623 from the claude-review findings: - engine.go: add os.Exit(1) after logger.Fatal on bad notifications.summary config. Logger.Fatal is interface-defined and not guaranteed to exit on every implementation (NoOpLogger.Fatal is a no-op), so force termination rather than risk falling through to the deterministic-fallback path. - backup.go: remove the timestamp dir when os.Create or db.Backup fails, so a failed/partial backup no longer counts toward backupRetention and prunes a valid backup on the next success. - summarizer.go: note in the error + a comment that the context-memory provider name is a legacy identifier (endpoint now points at Studio). - add summarizer_config_test.go: table-driven unit test covering the construction contract (disabled, unsupported provider, empty endpoint/key, fully configured, case-insensitive provider).
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
Repoints the workflow-summary endpoint from the deprecated context-memory service to Studio, which now hosts
/api/summarizewith the identical request/response contract.ContextAPIURLdefault →https://app.avaprotocol.org(client appends/api/summarize).contextMemory*request/response types deserialize Studio's response as-is.NotificationsSummary.APIEndpoint(avs-infra) + rotatedAPIKey.Companion PRs
feat/summarizer-from-context-memory(hosts the ported summarizer) — AvaProtocol/studio#1244feat/summarizer-endpoint-studio(staging config + key rotation)Depends on the Studio PR being deployed and
SUMMARIZE_AUTH_TOKENset before flipping prod config. The AVS deterministic fallback covers any gap.🤖 Generated with Claude Code