Skip to content

[feat] discover_triggers: 30-60s to <1s via a cached trigger catalog - #5189

Merged
mmabrouk merged 4 commits into
big-agentsfrom
docs/trigger-discovery-catalog-plan
Jul 10, 2026
Merged

[feat] discover_triggers: 30-60s to <1s via a cached trigger catalog#5189
mmabrouk merged 4 commits into
big-agentsfrom
docs/trigger-discovery-catalog-plan

Conversation

@mmabrouk

@mmabrouk mmabrouk commented Jul 9, 2026

Copy link
Copy Markdown
Member

Context

discover_triggers took 30 to 60 seconds while discover_tools answered in under a second. The trigger side had no semantic search to lean on, so it emulated one by paging the Composio catalog with keywords: up to 60 sequential, uncached HTTP calls per use case (integration searches, per-integration event searches, a detail fetch). Three use cases meant up to 180 round trips at ~0.55 s each.

The whole Composio trigger catalog is only 351 events across 41 toolkits, and every list item already carries its config schema and sample payload. So this PR fetches the catalog once, caches it, and scores in memory. The design was reviewed and approved on this PR first (the docs under docs/design/agent-workflows/projects/trigger-discovery-catalog/); the implementation now sits on top, with the five CodeRabbit review comments folded in.

Measured on the live EE dev stack: cold call 5.06 s (one 8-page catalog fetch), warm calls 0.66 to 1.11 s with zero Composio catalog calls. Before: 30 to 60 s on every call.

Changes

Catalog fetch (providers/composio/catalog.py): new list_all_events() on the catalog client pages GET /triggers_types (50 per page, Composio's cap) to exhaustion into a TriggerCatalogEventsSnapshot (all events plus a toolkit slug-to-display-name map). Guards: 50-page cap with a warning log when it truncates, and a repeated-cursor break.

Cache (service.py): _cached_event_catalog() mirrors the tools-side _cached_search: Redis namespace triggers:catalog, provider-only key (project-agnostic on purpose; connection state stays per-project and fresh), TTL COMPOSIO_CATALOG_CACHE_TTL_SECONDS (default 24 h). The whole fetch runs under one asyncio.wait_for deadline, COMPOSIO_CATALOG_FETCH_DEADLINE_SECONDS (default 30 s), converted to the domain AdapterError so one stalled page cannot stack per-page timeouts.

Discovery (service.py): discover_triggers loads the snapshot once per request; _discover_events_for_use_case became a pure static function scoring all 351 events in memory with the existing scorer. The keyword paginators (_candidate_integrations, _candidate_events) and the per-primary get_event call are deleted; trigger_config and payload come straight from the snapshot.

Per-request Composio catalog calls, before and after:

Before:  up to 60 per use case  (5 integration searches + 10x5 event searches + 1 detail)
After:   0 warm  /  8 pages once per deployment per 24 h

Scorer tie-break (service.py, found in live verification): with the provider's relevance ordering gone, equal-score ties broke by catalog order, and "when a new GitHub issue is created" surfaced the ISSUE_COMMENT event over ISSUE_CREATED (both scored 26). _match_signal now adds +4 per consecutive pair of use-case terms found adjacent in the normalized event key or name ("issue created" hits ISSUE_CREATED, not ISSUE_COMMENT_CREATED). Terms matching the integration's own key or name are excluded from pairs; a first version without that exclusion made "slack message" reward SLACK_MESSAGE_REACTION_ADDED, since the integration name prefixes every key. Both cases are pinned by unit tests with the wrong event listed first.

The wire contract is unchanged: same request, same TriggerCapabilitiesResult, router untouched.

Scope / risk

  • Catalog browse routes (list_events, get_event) stay on the live adapter, verified regression-free. Serving them from the snapshot is a follow-up.
  • The per-project connection-state joins are unchanged (still one live get_integration per surfaced, not-ready integration; bounded and needed for freshness).
  • With AGENTA_API_CACHING_ENABLED=false every request pays one full crawl (~4-5 s) instead of a cache read. Still bounded by the deadline; no crash.
  • Catalog staleness is at most the 24 h TTL; a removed trigger fails loudly at create_subscription time, which validates against the live provider.
  • Ranking changes only through the adjacency bonus; the primary-evidence gate and no-match path are untouched and covered by the existing tests.
  • Not vendored as a JSON file on purpose: Composio ToS §3 excludes derivative/commercial use of platform contents (see research.md §6). Runtime fetch with our own key is ordinary API usage.

Tests

  • 194 unit tests green in oss/tests/pytest/unit/triggers/ + unit/tools/, including new ones: pagination and cursor-guard for list_all_events, cache hit skips the adapter, cache miss writes with the configured TTL, deadline converts to AdapterError, one-fetch-per-request memoization, and the two ranking pins (GitHub, Slack).
  • Full unit lane green except 6 pre-existing failures in secrets/test_dtos.py (SSRF URL rejection), unrelated to this change.
  • Live verification on the EE dev stack: timings above, plus Redis key triggers:catalog:provider:composio observed at TTL ~86400, byte-identical responses across warm calls, and the browse route returning 200 with 20 events.

How to QA

Prerequisites: the EE dev stack (load-env hosting/docker-compose/ee/.env.ee.dev.local + run.sh --ee --dev) with COMPOSIO_API_KEY set. Mint credentials via POST /api/admin/simple/accounts/ (the accounts.py fixture pattern).

Steps:

  1. Clear the catalog cache to force a cold call: in the redis-volatile container, redis-cli --scan --pattern '*triggers:catalog*' and DEL the service key.
  2. POST /api/triggers/discover with {"use_cases": ["when a new GitHub issue is created", "new Slack message in a channel"]} and your ApiKey header. Time it.
  3. Repeat the same call twice more.
  4. Watch the api container logs across the calls.

Expected result: call 1 completes in single-digit seconds and logs one [composio] list_all_events items=351 toolkits=41; calls 2-3 return in under ~1 s with no Composio fetch lines. The GitHub capability's primary is GITHUB_ISSUE_CREATED_TRIGGER, the Slack one SLACKBOT_CHANNEL_MESSAGE_RECEIVED, both with non-null trigger_config.

Automated tests:

cd api && uv run --no-sync pytest oss/tests/pytest/unit/triggers/ -q

Edge cases: the discover response for a nonsense use case must still return the no-match note rather than an arbitrary event (covered by tests). One environment gotcha found during QA: uvicorn watchfiles on the dev stack does not react to host-side writes under api/oss/src/core/triggers/, so live-testing an edit there needs a forced reload (a byte-identical rewrite of oss/src/utils/env.py works); details in status.md.

https://claude.ai/code/session_01LJaJ4hpJfTPULrnqKqtTHg

@mmabrouk

mmabrouk commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@vercel

vercel Bot commented Jul 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agenta-documentation Ready Ready Preview, Comment Jul 10, 2026 10:16am

Request Review

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR implements cached full-catalog trigger discovery: it fetches Composio events once, caches the snapshot in Redis, scores events in memory, adds integration filtering and ranking changes, and preserves existing discovery contracts and browse routes. It also adds configuration, tests, evaluation tooling, agent guidance, and design documentation.

Changes

Trigger Catalog Discovery

Layer / File(s) Summary
Catalog snapshot and pagination
api/oss/src/core/triggers/dtos.py, api/oss/src/core/triggers/providers/composio/catalog.py, api/oss/tests/pytest/unit/triggers/test_triggers_catalog_client.py
Adds the catalog snapshot DTO and guarded cursor pagination that aggregates parsed events and integration display names while wrapping HTTP failures.
Cached discovery and scoring
api/oss/src/core/triggers/service.py
Loads a provider-scoped catalog snapshot with Redis caching and a fetch deadline, then filters, scores, deduplicates, ranks, and returns event details from cached data.
Configuration and behavioral validation
api/oss/src/utils/env.py, api/oss/tests/pytest/unit/triggers/*
Adds cache and deadline settings and validates snapshot wiring, cache behavior, timeout handling, fetch counts, and discovery ranking cases.
Design research and implementation record
docs/design/agent-workflows/projects/trigger-discovery-catalog/{README.md,context.md,plan.md,research.md,status.md}
Records catalog measurements, implementation decisions, scoring experiments, verification results, and rollout status.
Offline discovery evaluation
docs/design/agent-workflows/projects/trigger-discovery-catalog/scripts/discovery_eval.py
Adds a script that loads scorer logic, evaluates catalog queries, and reports ranking and false-positive metrics.
Agent-facing discovery guidance
sdks/python/agenta/sdk/agents/{adapters/agenta_builtins.py,platform/op_catalog.py}
Updates instructions and schema descriptions for validating matches, browsing alternatives, and handling no-match results.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant TriggersService
  participant Redis
  participant ComposioCatalog
  Client->>TriggersService: discover_triggers
  TriggersService->>Redis: read provider catalog
  Redis-->>TriggersService: cached snapshot or miss
  TriggersService->>ComposioCatalog: list_all_events on miss
  ComposioCatalog-->>TriggersService: paginated catalog snapshot
  TriggersService->>Redis: store snapshot with TTL
  TriggersService->>TriggersService: score and rank events in memory
  TriggersService-->>Client: discovered events and alternatives
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.79% which is insufficient. The required threshold is 60.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise and accurately summarizes the cached trigger-catalog latency improvement.
Description check ✅ Passed The description is clearly related to the trigger discovery caching and scoring changes.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/trigger-discovery-catalog-plan

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a846a837-a911-4e62-81f9-262a2b2d9c8e

📥 Commits

Reviewing files that changed from the base of the PR and between ead0700 and 7415e2d.

📒 Files selected for processing (5)
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/README.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/context.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/plan.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/research.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/status.md

Comment thread docs/design/agent-workflows/projects/trigger-discovery-catalog/research.md Outdated
Comment thread docs/design/agent-workflows/projects/trigger-discovery-catalog/status.md Outdated

@mmabrouk mmabrouk left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mmabrouk mmabrouk changed the title [docs] Plan: cached-catalog trigger discovery (30-60s to <1s) [feat] discover_triggers: 30-60s to <1s via a cached trigger catalog Jul 10, 2026
@mmabrouk
mmabrouk marked this pull request as ready for review July 10, 2026 00:12
@dosubot dosubot Bot added Backend enhancement New feature or request python Pull requests that update Python code tests size:XL This PR changes 500-999 lines, ignoring generated files. labels Jul 10, 2026

@mmabrouk mmabrouk left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation landed on this PR (commits be2d33e docs + 7773fc2 code). Inline notes below walk the diff in review order: cache seam, scorer change, crawl, config, tests. Live numbers and the full QA path are in the PR body.

)

async def _discover_events_for_use_case(
async def _cached_event_catalog(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review guide 1/5 — the cache seam. This mirrors ToolsService._cached_search one-to-one: provider-only key (project-agnostic on purpose, connection state is joined fresh per project downstream), namespace triggers:catalog, TTL from COMPOSIO_CATALOG_CACHE_TTL_SECONDS (24h default). The asyncio.wait_for below is the whole-fetch deadline from the design review: one stalled page can no longer stack the 8 per-page 30s timeouts; it converts to the domain AdapterError, same as any provider failure on the old path. The shared cache helper's built-in SET-NX lock single-flights concurrent cold callers for free.

# "slack message" would be a spurious adjacency. Score-only; matched_terms and
# exact_phrase stay untouched.
integration_name = str(integration.name or "").lower()
content_terms = [

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review guide 2/5 — the only scoring change. Base term weights are untouched; this adds +4 per consecutive pair of use-case terms found adjacent in the normalized event key/name. It replaces the relevance ordering the paged provider search used to give us on ties. Two subtleties both came from live verification: (1) without it, "when a new GitHub issue is created" surfaced ISSUE_COMMENT_CREATED over ISSUE_CREATED (both base-score 26, catalog order broke the tie); (2) integration-name terms are excluded from pairs because they prefix every event key of that integration — a first version rewarded SLACK_MESSAGE_REACTION_ADDED for "slack message". Both cases are pinned in tests with the wrong event listed first.

Comment thread api/oss/src/core/triggers/providers/composio/catalog.py
Comment thread api/oss/src/utils/env.py
os.getenv("COMPOSIO_WEBHOOK_REPLAY_WINDOW_SECONDS") or 300
)
# Full trigger-types catalog: project-agnostic cache TTL + whole-fetch deadline.
catalog_cache_ttl_seconds: int = int(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review guide 4/5 — the two knobs. TTL bounds staleness (a trigger removed by Composio inside the window fails loudly at create_subscription, which validates against the live provider), and the deadline bounds a cold call when Composio is degraded. Both follow the existing ComposioConfig field style; nothing reads os.getenv outside env.py.

adapter.list_all_events.assert_awaited_once()


async def test_discover_triggers_adjacency_bonus_breaks_catalog_order_tie():

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review guide 5/5 — what the new tests pin. These two ranking tests place the WRONG event first in the snapshot, so they fail if the adjacency bonus (or its integration-term exclusion) regresses — stable sort alone cannot make them pass. Below them: cache-hit skips the adapter, cache-miss writes with the configured TTL, deadline converts to AdapterError, and one-fetch-per-request memoization (assert_awaited_once). Pagination + cursor-guard live in the new test_triggers_catalog_client.py.

@mmabrouk

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@mmabrouk

Copy link
Copy Markdown
Member Author

CI note: the unit-test jobs skipped on the PR because the code push landed while this was still a draft (the workflow gates on draft, and ready_for_review does not retrigger). I dispatched the same workflow on the branch: run 29059521501. Result: 1257 passed, and the only failures are pre-existing on big-agents, byte-identical files this PR does not touch (6 secrets SSRF-rejection tests in oss/tests/pytest/unit/secrets/test_dtos.py and 2 SDK agents tests: test_unknown_harness_is_permissive, test_run_selection_unknown_permission_falls_back_to_allow_reads). The same 8 failures appear on feat/pi-approval-parking's run 29058074329. All triggers and tools tests, including the 194 touched by this PR, pass. The PR checks will run normally on the next push or can be judged by the dispatched run.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1b10855c-a000-4187-9b92-cfc630381a7c

📥 Commits

Reviewing files that changed from the base of the PR and between 7415e2d and 7773fc2.

📒 Files selected for processing (11)
  • api/oss/src/core/triggers/dtos.py
  • api/oss/src/core/triggers/providers/composio/catalog.py
  • api/oss/src/core/triggers/service.py
  • api/oss/src/utils/env.py
  • api/oss/tests/pytest/unit/triggers/test_triggers_catalog_client.py
  • api/oss/tests/pytest/unit/triggers/test_triggers_discovery.py
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/README.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/context.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/plan.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/research.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/status.md
✅ Files skipped from review due to trivial changes (4)
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/context.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/research.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/README.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/plan.md

Comment on lines +177 to +197

# Adjacency bonus: consecutive CONTENT terms appearing side by side in the event
# key/name break score ties the paged provider search used to resolve via its own
# relevance ordering (e.g. "issue created" prefers ISSUE_CREATED over
# ISSUE_COMMENT_CREATED). Integration-name tokens are excluded before pairing:
# they prefix every event key of that integration, so a bigram like
# "slack message" would be a spurious adjacency. Score-only; matched_terms and
# exact_phrase stay untouched.
integration_name = str(integration.name or "").lower()
content_terms = [
term
for term in terms
if term not in integration_key and term not in integration_name
]
normalized_key = _normalize_words(event.key)
normalized_name = _normalize_words(event.name)
for first, second in zip(content_terms, content_terms[1:]):
bigram = f"{first} {second}"
if bigram in normalized_key or bigram in normalized_name:
score += 4

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Adjacency bonus can pair terms that were never adjacent in the use case.

content_terms removes integration key/name tokens from terms before pairing. That shift means two terms separated only by the integration name become "adjacent" after filtering — e.g. use case "new GitHub issue created" → terms ["new","github","issue","created"] → after removing "github", content_terms = ["new","issue","created"], so the bigram "new issue" gets checked even though "new" and "issue" were never adjacent in the original text (both pinned regression tests below happen to pass only because the spurious bigram doesn't coincidentally match any tested event name).

Also, term not in integration_key/integration_name uses substring containment rather than exact match, so a legitimate content term that happens to be a substring of the integration name/key is wrongly excluded from pairing too.

Compute bigrams from the original ordered terms, and only skip a pair when one endpoint IS (not "contains") the integration key/name:

🐛 Proposed fix
     integration_name = str(integration.name or "").lower()
-    content_terms = [
-        term
-        for term in terms
-        if term not in integration_key and term not in integration_name
-    ]
     normalized_key = _normalize_words(event.key)
     normalized_name = _normalize_words(event.name)
-    for first, second in zip(content_terms, content_terms[1:]):
+
+    def _is_integration_term(term: str) -> bool:
+        return term == integration_key or term == integration_name
+
+    for first, second in zip(terms, terms[1:]):
+        if _is_integration_term(first) or _is_integration_term(second):
+            continue
         bigram = f"{first} {second}"
         if bigram in normalized_key or bigram in normalized_name:
             score += 4
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Adjacency bonus: consecutive CONTENT terms appearing side by side in the event
# key/name break score ties the paged provider search used to resolve via its own
# relevance ordering (e.g. "issue created" prefers ISSUE_CREATED over
# ISSUE_COMMENT_CREATED). Integration-name tokens are excluded before pairing:
# they prefix every event key of that integration, so a bigram like
# "slack message" would be a spurious adjacency. Score-only; matched_terms and
# exact_phrase stay untouched.
integration_name = str(integration.name or "").lower()
content_terms = [
term
for term in terms
if term not in integration_key and term not in integration_name
]
normalized_key = _normalize_words(event.key)
normalized_name = _normalize_words(event.name)
for first, second in zip(content_terms, content_terms[1:]):
bigram = f"{first} {second}"
if bigram in normalized_key or bigram in normalized_name:
score += 4
# Adjacency bonus: consecutive CONTENT terms appearing side by side in the event
# key/name break score ties the paged provider search used to resolve via its own
# relevance ordering (e.g. "issue created" prefers ISSUE_CREATED over
# ISSUE_COMMENT_CREATED). Integration-name tokens are excluded before pairing:
# they prefix every event key of that integration, so a bigram like
# "slack message" would be a spurious adjacency. Score-only; matched_terms and
# exact_phrase stay untouched.
integration_name = str(integration.name or "").lower()
normalized_key = _normalize_words(event.key)
normalized_name = _normalize_words(event.name)
def _is_integration_term(term: str) -> bool:
return term == integration_key or term == integration_name
for first, second in zip(terms, terms[1:]):
if _is_integration_term(first) or _is_integration_term(second):
continue
bigram = f"{first} {second}"
if bigram in normalized_key or bigram in normalized_name:
score += 4

mmabrouk added 4 commits July 10, 2026 10:32
…paging Composio per use case

discover_triggers emulated search with up to 60 sequential uncached Composio
calls per use case (30-60s wall time). The full trigger catalog is only 351
events across 41 toolkits, so the service now fetches it whole through the
adapter (list_all_events, 8 pages, page-cap + repeated-cursor guards), caches
it in Redis for 24h under a project-agnostic key (triggers:catalog), and
scores every use case in memory. One asyncio.wait_for deadline bounds the
whole fetch. Cold call measured 5.06s, warm 0.7-1.1s, wire contract unchanged.

Scoring keeps the existing term weights and adds a content-term adjacency
bonus to replace the provider's relevance ordering on ties; terms matching
the integration's own key/name are excluded from pairs (else 'slack message'
rewards the reaction event). Both live-found ranking cases are pinned by
unit tests.

New env knobs: COMPOSIO_CATALOG_CACHE_TTL_SECONDS (86400),
COMPOSIO_CATALOG_FETCH_DEADLINE_SECONDS (30).

Claude-Session: https://claude.ai/code/session_01LJaJ4hpJfTPULrnqKqtTHg
…form restriction, deprecated demotion

Validated by an 891-query offline eval (committed as scripts/discovery_eval.py in the
design workspace): browse-shaped queries went from 25% empty / 14% wrong toolkit to
100% right toolkit; templated ground truth holds at 98% top-1 / 100% top-4; deprecated
events no longer surface. Meta/question words become stopwords; a use case naming a
platform is restricted to it; DEPRECATED descriptions get -25; the no-match path
returns the closest events with an honest capped-list note. Tool description and
build-an-agent skill text updated to match.

Claude-Session: https://claude.ai/code/session_01LJaJ4hpJfTPULrnqKqtTHg
@mmabrouk
mmabrouk force-pushed the docs/trigger-discovery-catalog-plan branch from 7773fc2 to 0b79a52 Compare July 10, 2026 10:15
@dosubot dosubot Bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Jul 10, 2026
@mmabrouk

Copy link
Copy Markdown
Member Author

Follow-up commit 0b79a52: discovery quality pass, validated by an 891-query eval

A live check after the cache landed showed a second problem, separate from latency: what agents actually get back. "slack triggers" returned nothing at all. "what events can gmail trigger on" returned a WhatsApp event as the best match, because "what" is a substring of WHATSAPP. "discord message received" returned Slack events even though a Discord trigger exists.

Instead of patching cases one by one, this commit adds an offline eval (docs/design/agent-workflows/projects/trigger-discovery-catalog/scripts/discovery_eval.py) that extracts the real scoring functions from service.py via AST and replays them over the real catalog with 891 realistic queries. Nine scorer variants were A/B tested on that corpus. Four changes won and shipped together:

  1. Meta and question words become stopwords ("triggers", "webhook", "what", "can", ...). They add noise and collide with catalog substrings.
  2. Naming a platform restricts results to it. An event from the wrong platform is unusable no matter how well its words match. "discord message received" now returns the Discord trigger.
  3. Deprecated events lose 25 points. Live replacements always outrank them now.
  4. No-match returns the closest events instead of an empty list, with a note that says the list is capped by limit_alternatives. "slack triggers" now returns the Slack menu.

Measured on the eval corpus: browse-shaped queries went from 25% fully empty and 14% wrong toolkit to 100% right toolkit and 0% empty. The 686 templated ground-truth queries hold at 98% top-1 and 100% top-4, so nothing regressed. Both previously pinned ranking cases still pass. Variants that measured worse (plural matching, treating "new" as content, down-weighting descriptions) are documented as rejected in research.md §7 with the case each one broke.

The same pass fixes the other layers an agent touches: the discover_triggers tool description now says matching is keyword search, tells the agent to verify the integration and description before wiring an event, and explains that results are always capped so an absent event is not proof it does not exist. The build-an-agent skill (step 7 in agenta_builtins.py) carries the same guidance.

Verified: 106 triggers unit tests green (4 new pins), the committed eval reproduces the numbers against the final code, and all the cases above were re-run live on the dev stack (warm 0.7s).

Reviewed by a staff-level pass; its one required fix (the browse wording overclaimed "full list" while results are capped at limit_alternatives) is folded in, plus its nits.

https://claude.ai/code/session_01LJaJ4hpJfTPULrnqKqtTHg

@mmabrouk

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@mmabrouk

Copy link
Copy Markdown
Member Author

CI note for commit 0b79a52: the three red unit jobs are identical to the base big-agents run at the merge-base SHA 41eacd0 (run 29080056606 fails the same api/sdk/web jobs). api: 6 pre-existing secrets SSRF failures, 1264 passed including all triggers tests. sdk: 2 pre-existing permission-default failures, 1758 passed. web: 22 pre-existing agenta-entities failures where unit tests receive a live 404 HTML page (no web files in this PR). Python/TS lint and format, services unit tests, and runner suites are green.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
docs/design/agent-workflows/projects/trigger-discovery-catalog/scripts/discovery_eval.py (1)

178-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use the extracted _has_primary_evidence instead of inlining its logic.

_has_primary_evidence is in WANTED and extracted to ns, yet discover manually replicates its gate using _match_signal and _DISCOVERY_MIN_PRIMARY_TERMS. If _has_primary_evidence gains additional checks, the eval script silently drifts and reports inaccurate metrics — exactly the copy-drift the AST extraction approach was designed to prevent.

♻️ Proposed fix
     surfaced = matches[: 1 + max(limit_alternatives, 0)]
-    if surfaced:
-        _score, matched_terms, exact = ns["_match_signal"](
-            use_case=use_case, event=surfaced[0][1], integration=surfaced[0][2]
-        )
-        if not (exact or matched_terms >= ns["_DISCOVERY_MIN_PRIMARY_TERMS"]):
-            surfaced = []
+    if surfaced and not ns["_has_primary_evidence"](
+        use_case=use_case, event=surfaced[0][1], integration=surfaced[0][2]
+    ):
+        surfaced = []

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 169d1362-6545-450a-9af5-1377418beb57

📥 Commits

Reviewing files that changed from the base of the PR and between 7773fc2 and 0b79a52.

📒 Files selected for processing (14)
  • api/oss/src/core/triggers/dtos.py
  • api/oss/src/core/triggers/providers/composio/catalog.py
  • api/oss/src/core/triggers/service.py
  • api/oss/src/utils/env.py
  • api/oss/tests/pytest/unit/triggers/test_triggers_catalog_client.py
  • api/oss/tests/pytest/unit/triggers/test_triggers_discovery.py
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/README.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/context.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/plan.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/research.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/scripts/discovery_eval.py
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/status.md
  • sdks/python/agenta/sdk/agents/adapters/agenta_builtins.py
  • sdks/python/agenta/sdk/agents/platform/op_catalog.py
✅ Files skipped from review due to trivial changes (3)
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/README.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/context.md
  • docs/design/agent-workflows/projects/trigger-discovery-catalog/plan.md
🚧 Files skipped from review as they are similar to previous changes (5)
  • api/oss/tests/pytest/unit/triggers/test_triggers_catalog_client.py
  • api/oss/src/utils/env.py
  • api/oss/src/core/triggers/dtos.py
  • api/oss/src/core/triggers/providers/composio/catalog.py
  • api/oss/src/core/triggers/service.py

Comment on lines +761 to +773
async def test_cached_event_catalog_deadline_raises_adapter_error(monkeypatch):
service = _service()

async def _hang():
await asyncio.sleep(60)

service.adapter_registry.get = MagicMock(
return_value=SimpleNamespace(list_all_events=_hang)
)
monkeypatch.setattr(env.composio, "catalog_fetch_deadline_seconds", 0.01)

with pytest.raises(AdapterError):
await service._cached_event_catalog(provider_key="composio")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Add get_cache monkeypatch to the deadline test for isolation.

Unlike test_cached_event_catalog_hit_skips_adapter and test_cached_event_catalog_miss_fetches_once_and_caches, this test does not monkeypatch get_cache. If the real get_cache returns a cached value (e.g., from a prior test), the adapter is never called, AdapterError is never raised, and the test fails for the wrong reason. If get_cache raises or hangs, same outcome. Add a _miss returning None to guarantee the adapter path is reached.

🛡️ Proposed fix
 async def test_cached_event_catalog_deadline_raises_adapter_error(monkeypatch):
     service = _service()
+
+    async def _miss(**_kwargs):
+        return None
+
+    monkeypatch.setattr("oss.src.core.triggers.service.get_cache", _miss)
+
     async def _hang():
         await asyncio.sleep(60)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async def test_cached_event_catalog_deadline_raises_adapter_error(monkeypatch):
service = _service()
async def _hang():
await asyncio.sleep(60)
service.adapter_registry.get = MagicMock(
return_value=SimpleNamespace(list_all_events=_hang)
)
monkeypatch.setattr(env.composio, "catalog_fetch_deadline_seconds", 0.01)
with pytest.raises(AdapterError):
await service._cached_event_catalog(provider_key="composio")
async def test_cached_event_catalog_deadline_raises_adapter_error(monkeypatch):
service = _service()
async def _miss(**_kwargs):
return None
monkeypatch.setattr("oss.src.core.triggers.service.get_cache", _miss)
async def _hang():
await asyncio.sleep(60)
service.adapter_registry.get = MagicMock(
return_value=SimpleNamespace(list_all_events=_hang)
)
monkeypatch.setattr(env.composio, "catalog_fetch_deadline_seconds", 0.01)
with pytest.raises(AdapterError):
await service._cached_event_catalog(provider_key="composio")

@mmabrouk mmabrouk left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mmabrouk
mmabrouk merged commit cb63991 into big-agents Jul 10, 2026
17 of 23 checks passed
@mmabrouk mmabrouk mentioned this pull request Jul 10, 2026
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backend enhancement New feature or request python Pull requests that update Python code size:XXL This PR changes 1000+ lines, ignoring generated files. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant