Skip to content

refactor(openai-shim): extract tool conversion - #2006

Merged
kevincodex1 merged 5 commits into
Twigpine:mainfrom
jatmn:de-mono1-tools-and-json-parser
Jul 27, 2026
Merged

kevincodex1 merged 5 commits into
Twigpine:mainfrom
jatmn:de-mono1-tools-and-json-parser

Conversation

@jatmn

@jatmn jatmn commented Jul 18, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Extracts tool-schema conversion and raw JSON tool-call parsing into toolConversion.ts and rawToolCallParsing.ts.

The group owns schema sanitization, deferred-tool filtering, raw Gemini/text tool-call parsing, and bounded truncated-JSON repair. The façade delegates these cohesive tool boundaries.

Independent merge

  • Upstream base: 0effa0f42b6dbc6a4800e19f4b2d8d588269906b.
  • Fork branch: jatmn/openclaude:de-mono1-tools-and-json-parser.
  • Exact head: 554cef505b208e920d0b8e0a6a92878ed2d11a1f.
  • All 45 pairwise combinations and full forward/reverse ten-branch merges are clean.
  • Draft upstream PR.

Source-file budget

openaiShim.ts: +59 / -326 = 385 changed lines, below the 1,500-line cap; tests excluded.

Test migration

  • Monolithic test: +693 / -153; all three owned top-level test bodies are removed.
  • Focused files: toolConversion.test.ts (97 lines / 4 tests) and rawToolCallParsing.test.ts (78 lines / 7 tests).
  • Focused tool/parser, retained façade, and affected Ollama text-tool suites: 264 passed / 704 assertions.
  • Combined result: only three façade-contract tests remain in the monolithic test.

Validation

  • Diff check, typecheck, type-test check, and build passed.
  • Exact-head host bun run check: 7,194 passed / 2 skipped; only 10 pre-existing PowerShell governance failures remain on Linux.

Prepared according to CONTRIBUTING.md and AGENTS.md.

Summary by CodeRabbit

  • New Features

    • Improved recognition of tool calls embedded in free-form model text, including streamed content, “tool calls requested” sections, fenced JSON, and bare balanced-JSON fallbacks (with JSON repair for truncated objects).
  • Bug Fixes

    • Enhanced OpenAI-compatible tool schema conversion with correct strict-schema behavior, proper required handling (no accidental optional promotion), and Gemini-mode adjustments, including better support for special tool definitions.
  • Tests

    • Added Bun coverage for raw tool-call parsing, JSON repair/extraction, and tool/schema conversion edge cases.

@coderabbitai

coderabbitai Bot commented Jul 18, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change extracts OpenAI-compatible tool conversion and raw text tool-call parsing into dedicated modules, adds focused regression tests, and updates openaiShim.ts and related tests to use the new delegation boundaries.

Changes

OpenAI shim tool handling

Layer / File(s) Summary
Tool schema conversion and validation
src/services/api/openaiShim/toolConversion.ts, src/services/api/openaiShim/toolConversion.test.ts
Adds recursive schema normalization, OpenAI-compatible tool conversion, filtering, Agent required-field handling, and regression coverage.
Raw tool-call parsing primitives
src/services/api/openaiShim/rawToolCallParsing.ts, src/services/api/openaiShim/rawToolCallParsing.test.ts
Adds requested-tool parsing, truncated JSON repair, balanced JSON extraction, range stripping, and malformed-input tests.
Embedded text tool-call recovery
src/services/api/openaiShim/rawToolCallParsing.ts, src/services/api/openaiShim/rawToolCallParsing.test.ts
Parses fenced and bare JSON tool calls, normalizes arguments, deduplicates calls, assigns IDs, and validates recovery behavior.
Shim wrappers and import migration
src/services/api/openaiShim.ts, src/services/api/openaiShim.ollamaTextToolCalls.test.ts, src/services/api/openaiShim.test.ts
Replaces in-file implementations with delegated wrappers and updates test imports and extraction seams.

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

Possibly related PRs

Suggested reviewers: kevincodex1

🚥 Pre-merge checks | ✅ 5 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Risk Surface Disclosed ⚠️ Warning PR touches MCP-adjacent tool schema/parsing, but the description/review text never calls out the risk surface or says whether it is blocking. Add an explicit review note for the tool-schema/raw-parsing risk surface and state whether it is a blocker or non-blocking.
✅ Passed checks (5 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.
No Hidden Policy Change ✅ Passed No hidden policy/routing/telemetry/permission change found; the PR only extracts tool-schema and raw-tool parsing, and the façade keeps prior env/strict behavior.
Title check ✅ Passed The title is concise and matches the main refactor, though it omits the raw tool-call parsing extraction.
Description check ✅ Passed The description covers summary, migration, and validation, but it omits the template's Impact and Notes sections.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@jatmn jatmn self-assigned this Jul 18, 2026
@jatmn jatmn added the enhancement New feature or request label Jul 18, 2026
@jatmn
jatmn force-pushed the de-mono1-tools-and-json-parser branch from bb5b55d to 51fad44 Compare July 18, 2026 17:33
@jatmn
jatmn force-pushed the de-mono1-tools-and-json-parser branch from 51fad44 to 077e5dc Compare July 18, 2026 18:23
@jatmn
jatmn force-pushed the de-mono1-tools-and-json-parser branch 3 times, most recently from 554cef5 to 24443f9 Compare July 18, 2026 22:11
@jatmn
jatmn force-pushed the de-mono1-tools-and-json-parser branch from 24443f9 to d611660 Compare July 25, 2026 20:00

@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: 4

🤖 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 `@src/services/api/openaiShim/rawToolCallParsing.test.ts`:
- Around line 8-23: Add negative-case assertions to the test covering
couldBeRawToolCallsRequestedPrefix: verify unrelated prose and a diverging
partial prefix return false, while preserving the existing true assertion for
the valid “Tool calls” prefix.

In `@src/services/api/openaiShim/rawToolCallParsing.ts`:
- Line 17: Replace FENCED_TOOL_CALL_RE-based extraction with the existing
extractBalancedJson parser: detect fenced openings, calculate each opening brace
position, parse the balanced JSON, and require the closing fence immediately
after optional whitespace. Preserve the current fenced-block processing and
range behavior, skipping malformed or non-fenced matches.

In `@src/services/api/openaiShim/toolConversion.test.ts`:
- Around line 1-97: Add a focused test covering the non-strict Gemini path in
convertTools: pass a simple object schema with isGemini: true and
disableStrictTools: false, then assert the generated function parameters omit
additionalProperties rather than setting it to false. Keep the existing
strict-path tests unchanged.

In `@src/services/api/openaiShim/toolConversion.ts`:
- Around line 84-91: In the Agent handling block, prevent schema.required from
aliasing the caller’s input_schema.required array before appending keys. Clone
an existing required array into a new array, while retaining an empty array when
it is absent or invalid, then push missing message and subagent_type entries
onto that cloned array.
🪄 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: ASSERTIVE

Plan: Pro

Run ID: 80e52dbd-15bb-430e-82d7-ac6dd762e960

📥 Commits

Reviewing files that changed from the base of the PR and between 3f44341 and d611660.

📒 Files selected for processing (7)
  • src/services/api/openaiShim.ollamaTextToolCalls.test.ts
  • src/services/api/openaiShim.test.ts
  • src/services/api/openaiShim.ts
  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim/rawToolCallParsing.ts
  • src/services/api/openaiShim/toolConversion.test.ts
  • src/services/api/openaiShim/toolConversion.ts
💤 Files with no reviewable changes (1)
  • src/services/api/openaiShim.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: smoke-and-tests (24.11.x)
  • GitHub Check: smoke-and-tests (22)
  • GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

TypeScript code in this repository must use strict mode and ESM imports.

**/*.{ts,tsx}: Follow the existing code style and architectural patterns in touched TypeScript and TSX files.
Add or update tests when TypeScript or TSX changes affect behavior.
Review AI-generated TypeScript and TSX changes for correctness beyond compilation, consistency with repository architecture and style, unnecessary generated noise, and subtle bugs before submission.

Files:

  • src/services/api/openaiShim.ollamaTextToolCalls.test.ts
  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim/toolConversion.ts
  • src/services/api/openaiShim/toolConversion.test.ts
  • src/services/api/openaiShim/rawToolCallParsing.ts
  • src/services/api/openaiShim.ts
**/*

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*: Keep pull requests focused on one problem or feature; do not mix unrelated cleanup, fixes, features, or refactors into the same change.
Preserve existing repository patterns unless intentionally refactoring them, and prefer small, readable changes over broad rewrites.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
When changing provider behavior, avoid breaking third-party providers, test the exact provider/model path changed when possible, explicitly identify affected providers, and document limitations or follow-up work.
Do not assign or use provider tags; provider tags are controlled and applied by maintainers.
Run the relevant validation checks locally before submitting; CI-required checks include bun run check, bun run test:full, provider tests when applicable, typechecks, and bun run security:pr-scan. Web changes additionally require bun run web:typecheck and bun run web:build.
Dependency changes must have a concrete project benefit, such as fixing a bug, addressing a security issue, or supporting an approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, dependency stack, or significantly restructure dependencies without prior maintainer agreement.
Before implementing a new feature or other non-trivial change, open an issue to establish scope and alignment with the project roadmap.

Files:

  • src/services/api/openaiShim.ollamaTextToolCalls.test.ts
  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim/toolConversion.ts
  • src/services/api/openaiShim/toolConversion.test.ts
  • src/services/api/openaiShim/rawToolCallParsing.ts
  • src/services/api/openaiShim.ts

⚙️ CodeRabbit configuration file

**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.

Files:

  • src/services/api/openaiShim.ollamaTextToolCalls.test.ts
  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim/toolConversion.ts
  • src/services/api/openaiShim/toolConversion.test.ts
  • src/services/api/openaiShim/rawToolCallParsing.ts
  • src/services/api/openaiShim.ts
**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Run focused tests for changed behavior and ensure provider-specific changes include the relevant provider tests.

Files:

  • src/services/api/openaiShim.ollamaTextToolCalls.test.ts
  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim/toolConversion.test.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}

⚙️ CodeRabbit configuration file

{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.

Files:

  • src/services/api/openaiShim.ollamaTextToolCalls.test.ts
  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim/toolConversion.ts
  • src/services/api/openaiShim/toolConversion.test.ts
  • src/services/api/openaiShim/rawToolCallParsing.ts
  • src/services/api/openaiShim.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}

⚙️ CodeRabbit configuration file

{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.

Files:

  • src/services/api/openaiShim.ollamaTextToolCalls.test.ts
  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim/toolConversion.test.ts
🔇 Additional comments (6)
src/services/api/openaiShim.ts (2)

772-794: LGTM!


847-863: LGTM!

src/services/api/openaiShim.ollamaTextToolCalls.test.ts (1)

13-14: 📐 Maintainability & Code Quality

No issue: src/services/api/openaiShim.test.ts:6624-6628 already covers the façade-level sequence path and ollama_tc_* ids, so this retargeting does not leave the injected generator untested.

			> Likely an incorrect or invalid review comment.
src/services/api/openaiShim/toolConversion.ts (1)

1-83: LGTM!

Also applies to: 92-105

src/services/api/openaiShim/rawToolCallParsing.ts (1)

1-141: LGTM!

Also applies to: 163-239

src/services/api/openaiShim/rawToolCallParsing.test.ts (1)

1-79: 📐 Maintainability & Code Quality

No direct tests for parseTextToolCalls/extractBalancedJson/stripRanges in this dedicated suite.

This file only covers the raw "Tool calls requested:" prefix/repair functions, but the "Embedded text tool-call recovery" layer description states fenced/bare JSON tool-call parsing is "tested across valid and malformed inputs" and lists this file as covering it. Please confirm that suite exists in openaiShim.test.ts (not in this review batch) — if not, these are the most algorithmically subtle functions in the module (regex backtracking, balanced-brace counting, dedup) and warrant direct unit tests here rather than only indirect façade coverage.

Source: Path instructions

Comment thread src/services/api/openaiShim/rawToolCallParsing.test.ts
Comment thread src/services/api/openaiShim/rawToolCallParsing.ts Outdated
Comment thread src/services/api/openaiShim/toolConversion.test.ts
Comment thread src/services/api/openaiShim/toolConversion.ts
@jatmn
jatmn force-pushed the de-mono1-tools-and-json-parser branch from d611660 to 0b8c30f Compare July 26, 2026 15:45

@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: 3

🤖 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 `@src/services/api/openaiShim.ts`:
- Around line 772-775: Move the imports for convertToolsModule and
normalizeSchemaForOpenAIModule from their mid-file locations into the existing
top-of-file import block in openaiShim.ts, preserving their paths and aliases
without other changes.

In `@src/services/api/openaiShim/rawToolCallParsing.test.ts`:
- Around line 1-7: Extend the tests for the exported stripRanges helper in
rawToolCallParsing.test.ts with direct coverage of its sort-and-splice behavior:
provide unsorted ranges and assert that the corresponding text segments are
removed without leaking raw JSON or altering the remaining text.

In `@src/services/api/openaiShim/rawToolCallParsing.ts`:
- Around line 208-220: Update the bare-name handling in the parsed tool-call
logic so parsed.arguments is processed through the existing parseArguments
helper, matching the type:function branch. Preserve the empty-object fallback
for missing arguments and ensure stringified JSON arguments produce a
Record<string, unknown> for ParsedTextToolCall.
🪄 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: ASSERTIVE

Plan: Pro

Run ID: 5543bb63-527e-4ef3-978e-95672a669f0b

📥 Commits

Reviewing files that changed from the base of the PR and between d611660 and 0b8c30f.

📒 Files selected for processing (7)
  • src/services/api/openaiShim.ollamaTextToolCalls.test.ts
  • src/services/api/openaiShim.test.ts
  • src/services/api/openaiShim.ts
  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim/rawToolCallParsing.ts
  • src/services/api/openaiShim/toolConversion.test.ts
  • src/services/api/openaiShim/toolConversion.ts
💤 Files with no reviewable changes (1)
  • src/services/api/openaiShim.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: smoke-and-tests (22)
  • GitHub Check: smoke-and-tests (24.11.x)
  • GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

TypeScript code in this repository must use strict mode and ESM imports.

**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, including bun run typecheck and, when applicable, bun run typecheck:type-tests.

Files:

  • src/services/api/openaiShim.ollamaTextToolCalls.test.ts
  • src/services/api/openaiShim/toolConversion.test.ts
  • src/services/api/openaiShim/toolConversion.ts
  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim.ts
  • src/services/api/openaiShim/rawToolCallParsing.ts
**/*

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns in docs/integrations/overview.md and the focused guides under docs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Use bun install to install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.

Files:

  • src/services/api/openaiShim.ollamaTextToolCalls.test.ts
  • src/services/api/openaiShim/toolConversion.test.ts
  • src/services/api/openaiShim/toolConversion.ts
  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim.ts
  • src/services/api/openaiShim/rawToolCallParsing.ts

⚙️ CodeRabbit configuration file

**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.

Files:

  • src/services/api/openaiShim.ollamaTextToolCalls.test.ts
  • src/services/api/openaiShim/toolConversion.test.ts
  • src/services/api/openaiShim/toolConversion.ts
  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim.ts
  • src/services/api/openaiShim/rawToolCallParsing.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}

⚙️ CodeRabbit configuration file

{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.

Files:

  • src/services/api/openaiShim.ollamaTextToolCalls.test.ts
  • src/services/api/openaiShim/toolConversion.test.ts
  • src/services/api/openaiShim/toolConversion.ts
  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim.ts
  • src/services/api/openaiShim/rawToolCallParsing.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}

⚙️ CodeRabbit configuration file

{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.

Files:

  • src/services/api/openaiShim.ollamaTextToolCalls.test.ts
  • src/services/api/openaiShim/toolConversion.test.ts
  • src/services/api/openaiShim/rawToolCallParsing.test.ts
🔇 Additional comments (8)
src/services/api/openaiShim/toolConversion.ts (3)

12-57: LGTM!


84-91: Aliasing fix confirmed. The [...schema.required] clone resolves the earlier in-place mutation of the caller's input_schema, and the new test at toolConversion.test.ts Line 100 pins it.


3-10: 🗄️ Data Integrity & Integration

Remove this concern: function.strict is not emitted

convertTools only sends name, description, and parameters; strictness is enforced inside parameters, so there’s no request-payload change here.

			> Likely an incorrect or invalid review comment.
src/services/api/openaiShim/toolConversion.test.ts (1)

72-116: LGTM!

src/services/api/openaiShim/rawToolCallParsing.ts (1)

46-97: LGTM!

Also applies to: 99-141, 155-189

src/services/api/openaiShim/rawToolCallParsing.test.ts (1)

28-48: LGTM!

Also applies to: 76-91

src/services/api/openaiShim.ts (1)

777-794: LGTM!

Also applies to: 858-863

src/services/api/openaiShim.ollamaTextToolCalls.test.ts (1)

13-14: 🎯 Functional Correctness

The ID checks are order-independent. This suite only matches ollama_tc_\d+, so the extra raw parser import doesn’t make these assertions brittle.

			> Likely an incorrect or invalid review comment.

Comment thread src/services/api/openaiShim.ts Outdated
Comment thread src/services/api/openaiShim/rawToolCallParsing.test.ts
Comment thread src/services/api/openaiShim/rawToolCallParsing.ts

@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.

Caution

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

⚠️ Outside diff range comments (1)
src/services/api/openaiShim/rawToolCallParsing.ts (1)

161-166: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve valid tool calls followed by prose.

These guards discard a balanced tool call whenever remaining text is non-empty and does not start with {. Consequently, inputs such as a fenced call followed by “I’ll run that now” — or multiple fenced calls — return no calls. Accept the validated range and leave trailing prose for stripRanges; add regression coverage for fenced and bare calls followed by text.

🐛 Suggested fix
-    const after = text.slice(fenceEnd).trimStart()
-    if (after.length > 0 && !after.startsWith('{')) continue
     const range: [number, number] = [fenceStart, fenceEnd]

-    const after = text.slice(start + raw.length).trimStart()
-    if (after.length > 0 && !after.startsWith('{')) continue
     const range: [number, number] = [start, start + raw.length]

Also applies to: 183-184

🤖 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 `@src/services/api/openaiShim/rawToolCallParsing.ts` around lines 161 - 166,
Remove the trailing-text guards in the tool-call parsing flow around the
closing-fence handling and the corresponding bare-call path, so a successfully
validated balanced call is accepted regardless of remaining prose or additional
calls. Preserve the validated ranges so stripRanges can remove calls while
leaving trailing text intact, and add regression coverage for fenced and bare
calls followed by prose.
🤖 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.

Outside diff comments:
In `@src/services/api/openaiShim/rawToolCallParsing.ts`:
- Around line 161-166: Remove the trailing-text guards in the tool-call parsing
flow around the closing-fence handling and the corresponding bare-call path, so
a successfully validated balanced call is accepted regardless of remaining prose
or additional calls. Preserve the validated ranges so stripRanges can remove
calls while leaving trailing text intact, and add regression coverage for fenced
and bare calls followed by prose.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 85114f2f-cae0-477d-b8d8-dc97d29f685a

📥 Commits

Reviewing files that changed from the base of the PR and between 0b8c30f and 70fb888.

📒 Files selected for processing (3)
  • src/services/api/openaiShim.ts
  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim/rawToolCallParsing.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: typecheck
  • GitHub Check: smoke-and-tests (24.11.x)
  • GitHub Check: smoke-and-tests (22)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

TypeScript code in this repository must use strict mode and ESM imports.

**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, including bun run typecheck and, when applicable, bun run typecheck:type-tests.

Files:

  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim.ts
  • src/services/api/openaiShim/rawToolCallParsing.ts
**/*

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns in docs/integrations/overview.md and the focused guides under docs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Use bun install to install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.

Files:

  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim.ts
  • src/services/api/openaiShim/rawToolCallParsing.ts

⚙️ CodeRabbit configuration file

**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.

Files:

  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim.ts
  • src/services/api/openaiShim/rawToolCallParsing.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}

⚙️ CodeRabbit configuration file

{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.

Files:

  • src/services/api/openaiShim/rawToolCallParsing.test.ts
  • src/services/api/openaiShim.ts
  • src/services/api/openaiShim/rawToolCallParsing.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}

⚙️ CodeRabbit configuration file

{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.

Files:

  • src/services/api/openaiShim/rawToolCallParsing.test.ts
🔇 Additional comments (3)
src/services/api/openaiShim/rawToolCallParsing.ts (1)

38-44: LGTM!

Also applies to: 46-79, 81-97, 129-141, 194-247

src/services/api/openaiShim/rawToolCallParsing.test.ts (1)

1-8: LGTM!

Also applies to: 10-27, 29-49, 51-78, 80-133

src/services/api/openaiShim.ts (1)

113-113: LGTM!

Also applies to: 165-179, 791-804, 857-861

@jatmn
jatmn marked this pull request as ready for review July 26, 2026 16:08

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@kevincodex1
kevincodex1 merged commit 0652f05 into Twigpine:main Jul 27, 2026
5 checks passed
@jatmn
jatmn deleted the de-mono1-tools-and-json-parser branch July 27, 2026 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants