Feat/agent recommended additions - #1661
Open
SrihariLegend wants to merge 2 commits into
Open
SrihariLegend wants to merge 2 commits into
SrihariLegend wants to merge 2 commits into
Conversation
The insights pipeline computes two pieces of analysis every run and then discards them after folding them into the LLM prompt: - version_impact: cross-user layer/config correlation analysis (ClickHouse queries + layer-snapshot diffs) - registry_offer: the deterministic registry component shortlist shown to the model (build_signals + build_catalog) That is spent compute being thrown away. Persist both as nullable JSON columns on insight_reports so downstream consumers (duplicate detection, pull-time recommendations, governance drift signals) can read them without re-running the pipeline. Both are nullable: old reports predate the columns, and a run may legitimately produce neither. Also add a typed Pydantic schema (InsightAnalysisPayload) for the analytical payload, validated non-destructively on write: a structural mismatch is logged but never blocks the report write, so schema drift cannot break report generation. The existing untyped JSON blobs (metrics, narrative, aggregated_data) are unchanged — this is additive only. This is the "unconditionally worth it" subset of the broader InsightSnapshot refactor: it fixes real waste and types the payload without introducing a new entity, a cutover, or immutability discipline. The full snapshot externalization remains a separate, demand-gated decision. Review fixes (PR Observal#1660): - registry_offer serialization uses ``is not None`` rather than truthiness; CatalogOffer.__bool__ is bool(entries_by_type), so empty/disabled/failed offers are falsy and would have been dropped to None, losing the enabled / registry_has_components metadata that matters most in those cases. - ValidationError logging uses e.errors(include_input=False) and logs only loc/type/msg — never the raw input, which can carry LLM-generated narrative or session-derived text. - Migration docstring says "JSON" not "jsonb" (columns use sa.JSON(), matching the existing metrics/narrative/aggregated_data columns). - Add version_impact + registry_offer columns (migration 023) - Capture both in generator._run_pipeline return + _empty_report - Persist in batch.run_single_report + non-destructive validation - Add CatalogOffer.to_dict() to serialize the full offer - Expose both fields on InsightReportResponse (additive) - Tests for to_dict, payload schema, validation fallback, empty report, and the truthiness regression guard
The agent detail page and `observal pull` are pure install today: zero recommendations. This adds the first consumer of the persisted registry_offer analysis (PR #1) — a public endpoint and a "Recommended add-ons" rail on the agent detail Overview tab that surfaces public registry components the agent does not yet use but might benefit from, derived from observed usage. GET /api/v1/agents/{id}/insights/recommended-additions returns the latest completed insight report's deterministic component shortlist. Unlike the full insight report (edit-access-gated, exposes private telemetry), this surface exposes only public component references — type, id, qualified_name, name, description, category — so it is safe for anyone who can see the agent, including anonymous browsing. Degrades cleanly to an empty list when: no report exists for the agent, the report predates the registry_offer column (null), the offer was empty, or the feature was disabled at generation time. The frontend rail renders nothing on empty/error/loading — an empty rail is worse than no rail — so agents without reports look exactly as before. No migration, no pipeline change, no schema change to existing models: this reads PR #1's registry_offer column through a new read-only endpoint. - Add RecommendedAddition + RecommendedAdditionsResponse schemas - Add public GET /agents/{id}/insights/recommended-additions endpoint - Add useAgentRecommendedAdditions hook + insights.recommendedAdditions client - Add AgentRecommendedAdditions rail component - Render the rail on the agent detail Overview tab - Tests: offer entries returned, empty on no report / null offer / disabled / not-visible (404), malformed entries skipped
WalkthroughThe insight pipeline now persists version-impact and registry-offer data. A public endpoint returns validated recommendations for visible agents. The web application fetches and displays these recommendations in the agent overview. ChangesInsight recommendations
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AgentOverview
participant AgentRecommendedAdditions
participant useAgentRecommendedAdditions
participant insightsRecommendedAdditions
participant agent_recommended_additions
participant InsightReport
AgentOverview->>AgentRecommendedAdditions: pass agentId
AgentRecommendedAdditions->>useAgentRecommendedAdditions: query recommendations
useAgentRecommendedAdditions->>insightsRecommendedAdditions: fetch recommendations
insightsRecommendedAdditions->>agent_recommended_additions: GET agent recommendations
agent_recommended_additions->>InsightReport: select latest completed report
InsightReport-->>agent_recommended_additions: return registry_offer
agent_recommended_additions-->>insightsRecommendedAdditions: return normalized items
insightsRecommendedAdditions-->>useAgentRecommendedAdditions: return typed response
useAgentRecommendedAdditions-->>AgentRecommendedAdditions: provide cached result
AgentRecommendedAdditions-->>AgentOverview: render recommendation cards
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
web/src/components/registry/agent-recommended-additions.tsx (1)
9-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport through the frontend aggregators. Use
@/hooks/use-apiforuseAgentRecommendedAdditionsand@/lib/typesforRecommendedAddition.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/components/registry/agent-recommended-additions.tsx` around lines 9 - 10, Update the imports in the agent recommended additions component to use the frontend aggregator modules: import useAgentRecommendedAdditions from "`@/hooks/use-api`" and RecommendedAddition from "`@/lib/types`", removing the direct module imports.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@observal-server/schemas/insight_analysis.py`:
- Around line 27-30: Replace the structlog logger binding in
observal-server/schemas/insight_analysis.py lines 27-30 with the Loguru alias
optic, update the validation event at lines 83-83 to call optic.warning with
positional formatting and the sanitized errors, and update the validation
metadata logging in observal-server/services/insights/batch.py lines 269-275 to
use optic.info with positional arguments and no f-strings.
In `@web/src/hooks/use-insights-api.ts`:
- Around line 193-207: Update the report-completion invalidation flow in
useGenerateInsight to also invalidate the ["insights", "recommended-additions",
agentId] query used by useAgentRecommendedAdditions. Preserve the existing
reports invalidation and ensure the recommendation query refreshes immediately
after a report completes.
- Around line 201-205: Update the retry callback in the insights API query
options to use _count and cap retries at the configured limit of one retry for
non-404 errors. Continue returning false immediately when the error status is
404, and prevent persistent non-404 failures from retrying indefinitely.
---
Nitpick comments:
In `@web/src/components/registry/agent-recommended-additions.tsx`:
- Around line 9-10: Update the imports in the agent recommended additions
component to use the frontend aggregator modules: import
useAgentRecommendedAdditions from "`@/hooks/use-api`" and RecommendedAddition from
"`@/lib/types`", removing the direct module imports.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ce8a3885-6281-444e-87f7-536069b983bd
📒 Files selected for processing (15)
observal-server/alembic/versions/023_insight_analysis_payload.pyobserval-server/api/routes/agent/insights.pyobserval-server/models/insight_report.pyobserval-server/schemas/insight_analysis.pyobserval-server/schemas/insights.pyobserval-server/services/insights/batch.pyobserval-server/services/insights/generator.pyobserval-server/services/insights/registry_match.pytests/test_agent_recommended_additions.pytests/test_insight_analysis_payload.pyweb/src/components/registry/agent-recommended-additions.tsxweb/src/hooks/use-insights-api.tsweb/src/lib/api.tsweb/src/lib/types/admin.tsweb/src/pages/registry/agents/detail.tsx
Comment on lines
+27
to
+30
| import structlog | ||
| from pydantic import BaseModel, ConfigDict, Field, ValidationError | ||
|
|
||
| logger = structlog.get_logger(__name__) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use the required Loguru logger for validation events.
The new validation logs use structlog or logger. Use from loguru import logger as optic and positional formatting for these events.
observal-server/schemas/insight_analysis.py#L27-L30: replace thestructlogimport and logger binding with the required Loguru alias.observal-server/schemas/insight_analysis.py#L83-L83: emit the sanitized validation errors withoptic.warning("insight_payload_validation_failed errors={}", safe).observal-server/services/insights/batch.py#L269-L275: emit validation metadata withoptic.info(...)and positional arguments.
As per coding guidelines, use Loguru via from loguru import logger as optic and pass positional arguments without f-strings.
📍 Affects 2 files
observal-server/schemas/insight_analysis.py#L27-L30(this comment)observal-server/schemas/insight_analysis.py#L83-L83observal-server/services/insights/batch.py#L269-L275
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@observal-server/schemas/insight_analysis.py` around lines 27 - 30, Replace
the structlog logger binding in observal-server/schemas/insight_analysis.py
lines 27-30 with the Loguru alias optic, update the validation event at lines
83-83 to call optic.warning with positional formatting and the sanitized errors,
and update the validation metadata logging in
observal-server/services/insights/batch.py lines 269-275 to use optic.info with
positional arguments and no f-strings.
Source: Coding guidelines
Comment on lines
+193
to
+207
| export function useAgentRecommendedAdditions(agentId: string | undefined) { | ||
| return useQuery({ | ||
| queryKey: ["insights", "recommended-additions", agentId], | ||
| queryFn: () => insights.recommendedAdditions(agentId!), | ||
| enabled: !!agentId, | ||
| // The offer is derived from the latest completed report and changes only | ||
| // when a new report lands. Cheap to cache. | ||
| staleTime: 5 * 60_000, | ||
| retry: (_count, err: unknown) => { | ||
| // 404 = no report / agent not visible; not a retryable error. | ||
| const status = (err as { status?: number })?.status; | ||
| return status !== 404; | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether generating a new insight report invalidates the recommended-additions query cache.
set -euo pipefail
fd -t f 'use-insights-api.ts' -x cat -n {}
echo "--- invalidateQueries call sites touching insights keys ---"
rg -n -C 5 'invalidateQueries' web/src/hooks/use-insights-api.tsRepository: Observal/Observal
Length of output: 11093
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- recommended-additions hook usages ---"
rg -n -C 4 'useAgentRecommendedAdditions|recommended-additions|recommendedAdditions' web/src
echo "--- report generation and completion-related usages ---"
rg -n -C 5 'useGenerateInsight|generateInsight|insight.*(complete|completed|status)|report.*(complete|completed)|status === "(pending|running)"' web/src
echo "--- API definitions for insight report operations ---"
rg -n -C 8 'recommendedAdditions|generate:|generate\(|listReports|getReport' web/src/lib web/src/hooksRepository: Observal/Observal
Length of output: 40751
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- agent detail placement and lifecycle ---"
sed -n '500,635p' web/src/pages/registry/agents/detail.tsx
rg -n -C 8 'AgentRecommendedAdditions' web/src/pages/registry/agents/detail.tsx
echo "--- recommended additions component ---"
sed -n '1,180p' web/src/components/registry/agent-recommended-additions.tsx
echo "--- hook exports and duplicate implementations ---"
rg -n -C 6 'useGenerateInsight|useAgentRecommendedAdditions|useInsightReports' web/src/hooks/use-api.ts web/src/hooksRepository: Observal/Observal
Length of output: 13608
Invalidate recommended additions when a report completes.
useGenerateInsight invalidates only ["insights", "reports", agentId], and report polling does not invalidate ["insights", "recommended-additions", agentId]. The rail can therefore show stale or empty recommendations for up to five minutes after completion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/hooks/use-insights-api.ts` around lines 193 - 207, Update the
report-completion invalidation flow in useGenerateInsight to also invalidate the
["insights", "recommended-additions", agentId] query used by
useAgentRecommendedAdditions. Preserve the existing reports invalidation and
ensure the recommendation query refreshes immediately after a report completes.
Comment on lines
+201
to
+205
| retry: (_count, err: unknown) => { | ||
| // 404 = no report / agent not visible; not a retryable error. | ||
| const status = (err as { status?: number })?.status; | ||
| return status !== 404; | ||
| }, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- hook context ---'
sed -n '170,220p' web/src/hooks/use-insights-api.ts
printf '%s\n' '--- TanStack Query version and retry usage ---'
rg -n '"`@tanstack/react-query`"|retry:|useAgentRecommendedAdditions' package.json package-lock.json pnpm-lock.yaml yarn.lock web package.json 2>/dev/null | head -120
printf '%s\n' '--- related component error handling ---'
rg -n -C 5 'useAgentRecommendedAdditions|isError' web/src
printf '%s\n' '--- local documentation or type definitions for retry semantics ---'
rg -n -C 3 'retryDelay|retry.*failure|failureCount|retry.*3' . -g '*.ts' -g '*.tsx' -g '*.md' -g '*.json' | head -160Repository: Observal/Observal
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- query client defaults ---'
cat -n web/src/lib/query-client.ts
printf '%s\n' '--- recommended additions consumer ---'
cat -n web/src/components/registry/agent-recommended-additions.tsx | sed -n '1,90p'
printf '%s\n' '--- insights API error construction ---'
rg -n -C 8 'recommendedAdditions|status:|throw new|response\.status' web/src/lib web/src/hooks/use-insights-api.ts | head -180
printf '%s\n' '--- retry callback counts in the checked-in source ---'
rg -n -C 3 'retry:.*=>|retryDelay|failureCount|retryOnMount' web/srcRepository: Observal/Observal
Length of output: 16096
🌐 Web query:
TanStack Query v5 useQuery retry option retry function failureCount custom retry official documentation
💡 Result:
In TanStack Query v5, the retry option for useQuery can be configured as a boolean, a number, or a function to determine if and when a failed query should be retried [1][2]. When providing a function to the retry option, it takes two arguments: failureCount (the number of consecutive failures, starting at 0 for the first retry) and error (the error thrown by the query function) [1][2][3]. The function must return a boolean: return true to continue retrying, or false to stop [1][2]. Example of custom retry logic: const result = useQuery({ queryKey: ['todos'], queryFn: fetchTodoList, retry: (failureCount, error) => { // Example: Do not retry on a 404 error if (error.status === 404) { return false; } // Retry up to 3 times return failureCount < 3; }, }) Key behaviors: - failureCount: Starts at 0 for the first retry attempt [1][2]. - Defaults: By default, TanStack Query retries 3 times on the client and 0 times on the server [1]. - retryDelay: You can also customize the delay between retries using the retryDelay option, which accepts a number (in milliseconds) or a function that receives the retryAttempt count and the error to calculate the delay [1][2].
Citations:
- 1: https://tanstack.com/query/v5/docs/framework/react/reference/useQuery
- 2: https://tanstack.com/query/v5/docs/framework/react/guides/query-retries
- 3: https://dayvster.com/blog/react-query-retry/
Cap retries for non-404 errors.
The callback overrides the configured default of one retry and ignores _count, so persistent failures retry indefinitely. Bound _count to preserve the query client's retry limit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/hooks/use-insights-api.ts` around lines 201 - 205, Update the retry
callback in the insights API query options to use _count and cap retries at the
configured limit of one retry for non-404 errors. Continue returning false
immediately when the error status is 404, and prevent persistent non-404
failures from retrying indefinitely.
This branch has not been deployed
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
1 participant
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.
Purpose / Description
The agent detail page and
observal pullare pure install today: zero recommendations. A person browsing an agent gets no signal about what else they might benefit from. Meanwhile, the insights pipeline already computes a deterministic shortlist of registry components an agent doesn't use but might benefit from (registry_offer) — and as of #1660, that analysis is now persisted instead of discarded every run.This PR adds the first consumer of that persisted analysis: a public endpoint and a "Recommended add-ons" rail on the agent detail page that surfaces evidence-backed component suggestions derived from observed usage.
Fixes
Approach
A new read-only public endpoint + a frontend rail. No migration, no pipeline change, no schema change to existing models — it reads #1660's
registry_offercolumn.GET /api/v1/agents/{id}/insights/recommended-additions(public,optional_current_user) — loads the latest completed insight report for the agent and returns itsregistry_offercomponent shortlist as a flat list. Unlike the full insight report (edit-access-gated, exposes private session telemetry), this surface exposes only public component references —type,id,qualified_name,name,description,category— so it is safe for anyone who can see the agent, including anonymous browsing. Agent visibility is checked viacheck_listing_visibility_async; the offer entries are public registry components by construction (the recommender only returns approved+visible listings).AgentRecommendedAdditionsrail component — renders the shortlist on the agent detail Overview tab, reusing the visual pattern from the existingRecommendedForYourail (type badge, name link to component detail, description).itemswhen: no report exists for the agent, the report predates theregistry_offercolumn (null, from feat(insights): persist discarded run analysis #1660's no-backfill design), the offer was empty, or the feature was disabled at generation time. The rail returnsnullon empty/error/loading — an empty rail is worse than no rail — so agents without reports look exactly as before.This is deliberately the cheapest, highest-value first consumer of the persisted analysis: it delivers a feature that didn't exist anywhere in the product, it reuses already-spent compute, and it validates the "persist the analysis, then let consumers read it" pattern with a real reader.
How Has This Been Tested?
make test— 122 tests pass on this branch (new + existing insights, agent-scoped routes, access, registry-match, self-learn, retention).New tests in
tests/test_agent_recommended_additions.py(6):source_report_id+generated_at)registry_offeris null (old reports predate the column)enabled=False)Reproduce with:
Frontend:
tsc --noEmit(typecheck),eslint, and fullvite buildall pass clean. The rail renders nothing when there's no data, so the agent detail page is unchanged for agents without insight reports.make lintclean.Learning
This PR is the payoff for #1660's "persist the discarded analysis" change. #1660 stopped throwing away
version_impactandregistry_offerevery run but shipped no consumer — this is the first reader, proving a feature can use that analysis without touching the pipeline. The key design decision was keeping the endpoint narrow: it returns only the public component shortlist, never the report's narrative or metrics, which lets it be public (anyone browsing) rather than edit-access-gated like the full insight report. The "render nothing on empty" pattern (borrowed from the existingRecommendedForYourail) is what makes uneven report coverage acceptable — the rail fills in as reports accumulate rather than looking broken on agents without them.Checklist
feat: evidence-backed add-ons on agent detail(45 chars)Summary by CodeRabbit