Skip to content

fix(ai): align conversation execution and query tool contracts - #782

Merged
izadoesdev merged 1 commit into
stagingfrom
codex/conversation-policy-discovery
Sep 14, 2026
Merged

izadoesdev merged 1 commit into
stagingfrom
codex/conversation-policy-discovery

Conversation

@izadoesdev

@izadoesdev izadoesdev commented Sep 10, 2026

Copy link
Copy Markdown
Member

Conversational requests such as “Thanks, what is our retention?” could lose every tool through keyword filtering. MCP data calls also repeatedly supplied a preset and explicit dates together, while dashboard Balanced reasoning sent Anthropic options to the configured OpenAI model.

Use one native ToolLoopAgent setup for dashboard and shared MCP/Slack execution, derive provider options from the selected model, attribute billing to the actual model, and expose the existing analytics discovery/schema tools without keyword gating. Make optional internal MCP data inputs nullable on the wire and normalize them with Zod before the unchanged query planner. Keep strict generation; disabling it alone did not fix the observed failures. The dashboard default reasoning label now reflects provider defaults.

Validation:

  • Root lint, all 33 workspace typecheck tasks and the full 27-task test suite pass.
  • Six regression tests fail against clean staging and pass with the changes. Native SDK tests cover retries without repeating a completed mutation, cancellation, scope/callback/stopping preservation, nullable input normalization and conflicting-date rejection. Actual dashboard HTTP tests verify billing failures stop model execution and usage uses the executed model ID.
  • 48 live native-config MCP attempts: six frozen synthetic cases, two repeats, baseline plus three candidates, same configured Terra model and 45-second limits. Baseline and the first two candidates answer 0/10 data tasks; the final schema repair answers 6/10, with 2/2 greeting controls passing. Native date conflicts fall from 26 to zero. All four retention attempts still fail at discovery. Input tokens rise from 246,238 to 560,614 (includes cached inputs); sample median latency falls from 19.35s to 12.87s. These are diagnostic measurements, not a general efficiency claim. SQL/schema/profile alternatives were deliberately unavailable in the synthetic backend, so this measures supported builder workflows rather than all production strategies. Failed variants, timeouts, fixtures and raw traces remain in the task’s local evidence archive.
  • Four isolated live provider probes accept both old and corrected settings with correct arithmetic. Wire capture verifies the OpenAI option namespace; responses do not confirm effective reasoning effort or prove a quality gain.

Integration: rebased onto staging with the Autumn-only billing changes from #788. Server-derived included-chat access, legacy credit denial, rate limits, and usage accounting remain authoritative across dashboard, MCP, and Slack. Terra and Luna both preserve selected low/medium/high reasoning effort. The combined source passes fresh root lint, 33 type/build tasks, 27 test tasks, 52 focused native SDK tests and 11 actual API HTTP tests. A fresh configured review is requested on the final head. Broader tool exposure increases context cost; the discovery fallback from #783 is already on staging. No new database schema or investigation loop. AI-assisted implementation and review under the maintainer workflow.

@vercel

vercel Bot commented Sep 10, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated
dashboard Ready Ready Preview Sep 14, 2026 12:45pm UTC
databuddy-status Ready Ready Preview Sep 14, 2026 12:45pm UTC
1 Skipped Deployment
Project Deployment Actions Updated
documentation Skipped Skipped Sep 14, 2026 12:45pm UTC

@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: c423e2ae-6dbd-4861-899a-101b22d41ce8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@izadoesdev
izadoesdev force-pushed the codex/conversation-policy-discovery branch from f612504 to 8679bbb Compare September 14, 2026 12:43
@izadoesdev
izadoesdev marked this pull request as ready for review September 14, 2026 12:43
@vercel
vercel Bot temporarily deployed to Preview – documentation September 14, 2026 12:43 Inactive
@unkey-deploy

unkey-deploy Bot commented Sep 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Unkey Deploy

Name Status Preview Inspect Updated (UTC)
links (preview) Ready Visit Preview Inspect Sep 14, 2026 12:43pm

@izadoesdev

Copy link
Copy Markdown
Member Author

@greptileai Please review final head 8679bbb before staging merge. This conversation slice is rebased onto the merged native Autumn billing and query-discovery changes. It preserves server-derived billing access, fail-closed behavior, rate limits, and actual executed-model accounting while using one native conversation runner and validated nullable query inputs. Terra and Luna both retain requested OpenAI reasoning effort. Fresh root lint, 33 type/build tasks, 27 test tasks, 52 focused native tests and 11 actual API HTTP tests pass. Independent review confirmed all 20 paths match the tested integration and do not touch the merged Insights settlement fix.

@greptile-apps

greptile-apps Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Dashboard, MCP, and Slack now use one shared conversation agent with model-specific settings and full tool access. Query inputs accept strict-provider nulls, while billing records the model that actually ran.

  • Removes keyword-based tool filtering from MCP and Slack conversations.
  • Adds provider-aware reasoning settings for Terra, Luna, and Anthropic models.
  • Exposes query discovery and schema tools to analytics conversations.
  • Normalizes nullable query fields before the existing query planner runs.

Confidence Score: 4/5

The runtime changes look safe, but the new mock-based tests break an explicit repo rule and must change before merge.

The shared runner keeps the existing access, billing, retry, and tool checks. The only accepted issue is the banned mock test setup.

Files Needing Attention: packages/ai/src/ai/mcp/agent-query-schema.test.ts and packages/ai/src/ai/mcp/business-context-delivery.test.ts

Important Files Changed

Filename Overview
packages/ai/src/ai/agents/conversation.ts Adds the shared ToolLoopAgent setup. It keeps retries, tool scope, stop rules, callbacks, and Anthropic cache hints in one place.
packages/ai/src/ai/config/conversation-model.ts Chooses reasoning and cache options from the actual model ID. Current Terra, Luna, and Anthropic settings match their configured providers.
packages/ai/src/ai/mcp/agent-query-schema.ts Accepts null for optional get_data fields and turns null into the existing planner defaults. It also blocks filter fields that could change the target.
packages/ai/src/ai/mcp/run-agent.ts Moves MCP and Slack to the shared conversation runner and lets the model see the full tool set. Billing and access checks still happen before model work.
packages/ai/src/ai/mcp/agent-tools.ts Adds query discovery and schema tools, and uses the normalized get_data input schema. Existing website checks still guard data calls.
apps/api/src/routes/agent.ts Moves dashboard chat to the shared runner and bills usage with the executed model ID. Rate limits, billing access, and message checks remain before model execution.
packages/ai/src/ai/mcp/agent-query-schema.test.ts Covers nullable tool inputs and date conflicts, but adds synthetic mock data forbidden by the repo rule.
packages/ai/src/ai/mcp/business-context-delivery.test.ts Adds tool exposure and billing checks, but also adds synthetic mock tools forbidden by the repo rule.

Sequence Diagram

sequenceDiagram
    actor User
    participant API as Dashboard / MCP / Slack
    participant Access as Auth, rate, billing
    participant Config as Model config
    participant Agent as Shared conversation agent
    participant Model as Selected model
    participant Tools as Agent tools
    participant Query as Query planner
    participant Usage as Usage billing

    User->>API: Send conversation request
    API->>Access: Check identity, site access, rate, allowance
    alt Request is blocked
        Access-->>User: Return error before model work
    else Request is allowed
        API->>Config: Select model and provider options
        Config->>Agent: Build one native tool loop
        Agent->>Model: Send messages and all available tools
        opt Model calls get_data
            Model->>Tools: Call get_data with nullable fields
            Tools->>Tools: Normalize null to defaults
            Tools->>Access: Check website access
            Tools->>Query: Build and run valid queries
            Query-->>Model: Return rows and query errors
        end
        opt Model calls another tool
            Model->>Tools: Call discovery, read, or mutation tool
            Tools->>Access: Apply tool permission and confirmation checks
            Tools-->>Model: Return tool result
        end
        Model-->>Agent: Return answer and token usage
        Agent->>Usage: Bill the selected execution
        Agent-->>User: Return or stream answer
    end
Loading

Reviews (1): Last reviewed commit: "fix(ai): align conversation execution an..." | Re-trigger Greptile

Comment thread packages/ai/src/ai/mcp/agent-query-schema.test.ts
@izadoesdev
izadoesdev merged commit 1add298 into staging Sep 14, 2026
21 checks passed
@izadoesdev
izadoesdev deleted the codex/conversation-policy-discovery branch September 14, 2026 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant