Skip to content

chore: tool transforms + typegen#3978

Open
ehayes2000 wants to merge 2 commits into
mainfrom
tool-transforms
Open

chore: tool transforms + typegen#3978
ehayes2000 wants to merge 2 commits into
mainfrom
tool-transforms

Conversation

@ehayes2000

Copy link
Copy Markdown
Contributor
  • cleanup how we do schema transformation and validation on tools
  • add transforms to make tools openai compatible
  • remove unused schemas from swagger
  • remove unneeded anthropic model wrapper

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Refactoring
    • Modernized AI tool schema generation and validation system
    • Enhanced TypeScript type generation with improved code deduplication
    • Streamlined schema processing pipeline with stricter validation
    • Removed deprecated schema wrapping layer for cleaner code generation

Walkthrough

This PR refactors AI tool schema generation into two separate paths: ValidatedSchema for provider input validation and FrontendSchemas for TypeScript codegen. It replaces a combined schema approach with a modular, transform-based validation pipeline. The agent code is simplified by removing the AnthropicModel wrapper and using Anthropic's completion model directly. All toolset tests are updated to use the new generate_validated_input_schema API, and OpenAPI definitions are cleaned up to remove obsolete schema types.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title follows the conventional commits format with 'chore:' prefix and is under 72 characters (32 characters total). It clearly references the main changes: tool transforms and typegen work.
Description check ✅ Passed The description is directly related to the changeset, covering schema transformation cleanup, OpenAI compatibility transforms, swagger schema removal, and Anthropic model wrapper removal, which all align with the file changes.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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 and usage tips.

@github-actions

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
rust/cloud-storage/ai_toolset/src/schema/transform/additional_properties.rs (1)

1-41: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Blocked by repository rule: ai_toolset crate is marked immutable in project guidelines.

This change modifies rust/cloud-storage/ai_toolset/**, which current repository guidance explicitly disallows. Please confirm whether this policy has been superseded for this PR; otherwise this should not merge as-is.

As per coding guidelines: “rust/cloud-storage/ai_toolset/**/*.rs: You are not allowed to ever change this crate”.

🤖 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 `@rust/cloud-storage/ai_toolset/src/schema/transform/additional_properties.rs`
around lines 1 - 41, The PR changes files in the immutable ai_toolset crate
(additional_properties.rs) which violates the repository rule; either revert
these edits in AdditionalPropertiesFalse/is_object_schema/transform and restore
the original file, or move the new logic into a permitted crate (create a new
module with the AdditionalPropertiesFalse transform and is_object_schema helper,
then call/import it from the non-immutable crate), or obtain and document an
explicit exception from the repo owners before merging—do not merge changes to
rust/cloud-storage/ai_toolset/** without that approval.

Source: Coding guidelines

🧹 Nitpick comments (1)
rust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rs (1)

138-150: 💤 Low value

Transitive $ref ownership search is shallow.

find_owning_tool only searches the immediate definition of a tool's input/output for the target ref string. If the collided type is referenced indirectly (e.g., ToolInput -> SharedType -> CollidedType2), the ownership won't be traced through the intermediate SharedType definition.

The fallback at line 149 returns the def name itself, so this doesn't break functionality — the renamed type just won't be tool-prefixed in that edge case. This is acceptable for typegen aesthetics but worth noting if naming becomes important later.

🤖 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 `@rust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rs` around lines
138 - 150, find_owning_tool currently only checks a tool's immediate
input/output definition with Self::value_contains_ref, so transitive $ref chains
(e.g., ToolInput -> SharedType -> CollidedType2) are missed; update
find_owning_tool to perform a transitive search by walking referenced defs via
self.defs (e.g., BFS/DFS) starting from each tool.input and tool.output,
invoking value_contains_ref on each visited def and following any $ref targets
found in that def, use a visited set to avoid cycles, and return
tool.name.clone() as soon as the target ref is discovered.
🤖 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 `@rust/cloud-storage/ai_toolset/src/schema/validate/extract.rs`:
- Around line 1-29: This change touches an immutable crate (the Extract type and
its extract/new methods) which violates the repo rule; either revert the edits
in the Extract implementation and restore the original file, or move the new
functionality into an allowed crate/module: create a new module or crate,
implement the same Extract struct and its methods (new and extract) there,
update references where ValidationError, Extract::new and Extract::extract are
used, and add the new crate/module to Cargo.toml and imports; if you must keep
the change here instead, obtain an explicit exception from the repo owners
before merging.

In `@rust/cloud-storage/ai_toolset/src/toolset/tool_object/mod.rs`:
- Line 12: The change touched an immutable crate (the pub use in mod.rs exposing
SchemaRegistrar and ToolObject) which must not be edited; revert any edits to
rust/cloud-storage/ai_toolset/src/toolset/tool_object/mod.rs and instead
implement your changes outside this crate — for example, create a new crate or
module in an allowed path that depends on the ai_toolset crate and provides the
wrapper, extension trait, or re-export you need (referencing ToolObject and
SchemaRegistrar by name), or move any new logic into the consuming crate rather
than modifying mod.rs.

---

Outside diff comments:
In `@rust/cloud-storage/ai_toolset/src/schema/transform/additional_properties.rs`:
- Around line 1-41: The PR changes files in the immutable ai_toolset crate
(additional_properties.rs) which violates the repository rule; either revert
these edits in AdditionalPropertiesFalse/is_object_schema/transform and restore
the original file, or move the new logic into a permitted crate (create a new
module with the AdditionalPropertiesFalse transform and is_object_schema helper,
then call/import it from the non-immutable crate), or obtain and document an
explicit exception from the repo owners before merging—do not merge changes to
rust/cloud-storage/ai_toolset/** without that approval.

---

Nitpick comments:
In `@rust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rs`:
- Around line 138-150: find_owning_tool currently only checks a tool's immediate
input/output definition with Self::value_contains_ref, so transitive $ref chains
(e.g., ToolInput -> SharedType -> CollidedType2) are missed; update
find_owning_tool to perform a transitive search by walking referenced defs via
self.defs (e.g., BFS/DFS) starting from each tool.input and tool.output,
invoking value_contains_ref on each visited def and following any $ref targets
found in that def, use a visited set to avoid cycles, and return
tool.name.clone() as soon as the target ref is discovered.
🪄 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

Run ID: 6b32eb60-0beb-4187-8b81-b6ee19e60ae6

📥 Commits

Reviewing files that changed from the base of the PR and between f994203 and 724745a.

⛔ Files ignored due to path filters (3)
  • js/app/packages/service-clients/service-cognition/generated/schemas/index.ts is excluded by !**/generated/**
  • js/app/packages/service-clients/service-cognition/generated/schemas/toolSchema.ts is excluded by !**/generated/**
  • js/app/packages/service-clients/service-cognition/generated/schemas/toolSchemas.ts is excluded by !**/generated/**
📒 Files selected for processing (45)
  • js/app/packages/service-clients/service-cognition/openapi.json
  • js/app/scripts/generate-dcs-tools.ts
  • rust/cloud-storage/agent/src/agent_loop.rs
  • rust/cloud-storage/agent/src/anthropic_model.rs
  • rust/cloud-storage/agent/src/anthropic_model/test.rs
  • rust/cloud-storage/agent/src/completion.rs
  • rust/cloud-storage/agent/src/lib.rs
  • rust/cloud-storage/ai_tools/src/bin/gen_tool_schemas.rs
  • rust/cloud-storage/ai_tools/src/lib.rs
  • rust/cloud-storage/ai_tools/src/search/search_service/content.rs
  • rust/cloud-storage/ai_tools/src/search/search_service/name.rs
  • rust/cloud-storage/ai_toolset/src/schema/error.rs
  • rust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rs
  • rust/cloud-storage/ai_toolset/src/schema/generate.rs
  • rust/cloud-storage/ai_toolset/src/schema/generate/test.rs
  • rust/cloud-storage/ai_toolset/src/schema/mod.rs
  • rust/cloud-storage/ai_toolset/src/schema/phantom_tool.rs
  • rust/cloud-storage/ai_toolset/src/schema/transform/additional_properties.rs
  • rust/cloud-storage/ai_toolset/src/schema/transform/mod.rs
  • rust/cloud-storage/ai_toolset/src/schema/transform/nullify_optional.rs
  • rust/cloud-storage/ai_toolset/src/schema/transform/ref_siblings.rs
  • rust/cloud-storage/ai_toolset/src/schema/transform/required.rs
  • rust/cloud-storage/ai_toolset/src/schema/transform/rewrite_one_of.rs
  • rust/cloud-storage/ai_toolset/src/schema/transform/strip_unsupported.rs
  • rust/cloud-storage/ai_toolset/src/schema/validate/extract.rs
  • rust/cloud-storage/ai_toolset/src/schema/validate/mod.rs
  • rust/cloud-storage/ai_toolset/src/schema/validate/one_of.rs
  • rust/cloud-storage/ai_toolset/src/schema/validate/recursive.rs
  • rust/cloud-storage/ai_toolset/src/schema/validate/recursive/test.rs
  • rust/cloud-storage/ai_toolset/src/schema/validate/strict.rs
  • rust/cloud-storage/ai_toolset/src/toolset/tool_object/mod.rs
  • rust/cloud-storage/ai_toolset/src/toolset/tool_object/object.rs
  • rust/cloud-storage/ai_toolset/src/toolset/tool_object/tool_async.rs
  • rust/cloud-storage/ai_toolset/src/toolset/tool_object/util.rs
  • rust/cloud-storage/ai_toolset/src/toolset/types.rs
  • rust/cloud-storage/call/src/inbound/toolset/test.rs
  • rust/cloud-storage/channels/src/inbound/toolset/test.rs
  • rust/cloud-storage/chat/src/inbound/toolset/test.rs
  • rust/cloud-storage/document_cognition_service/src/api/swagger.rs
  • rust/cloud-storage/documents/src/inbound/toolset/test.rs
  • rust/cloud-storage/email/src/inbound/toolset/test.rs
  • rust/cloud-storage/notification/src/inbound/ai_tool/test.rs
  • rust/cloud-storage/properties/src/inbound/toolset/test.rs
  • rust/cloud-storage/soup/src/inbound/toolset/test.rs
  • rust/cloud-storage/teams/src/inbound/toolset/test.rs
💤 Files with no reviewable changes (7)
  • rust/cloud-storage/agent/src/anthropic_model/test.rs
  • rust/cloud-storage/ai_toolset/src/toolset/tool_object/object.rs
  • rust/cloud-storage/ai_toolset/src/toolset/tool_object/util.rs
  • rust/cloud-storage/agent/src/lib.rs
  • rust/cloud-storage/document_cognition_service/src/api/swagger.rs
  • rust/cloud-storage/agent/src/anthropic_model.rs
  • js/app/packages/service-clients/service-cognition/openapi.json

Comment thread rust/cloud-storage/ai_toolset/src/schema/validate/extract.rs
Comment thread rust/cloud-storage/ai_toolset/src/toolset/tool_object/mod.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant