Repository navigation
Use tool calling - #121
Use tool calling#121
Conversation
Reviewer's GuideSwitches the default extraction pathway from response_format to tool calling and plumbs an extraction_method option through configuration, LLM call, and pipeline; adds schema formatting for tools, adjusts failure handling, and extends the Pydantic schema slightly. Sequence diagram for the new extraction_method tool_call vs response_format flowsequenceDiagram
participant Pipeline as PerlaExtractPipeline
participant LLM as llm_call
participant LiteLLM as litellm_completion
participant Model as PerovskiteSolarCells
Pipeline->>LLM: create_text_completion(model_name, pdf_text, extraction_method)
LLM->>LLM: _create_text_completion(extraction_method)
LLM->>LLM: format_schema(PerovskiteSolarCells)
alt extraction_method == response_format
LLM->>LiteLLM: completion(response_format=PerovskiteSolarCells)
LiteLLM-->>LLM: resp
LLM->>Model: model_validate_json(resp.choices[0].message.content)
else extraction_method == tool_call
LLM->>LiteLLM: completion(tools, tool_choice, reasoning_effort='none')
LiteLLM-->>LLM: resp
LLM->>Model: model_validate_json(resp.choices[0].message.tool_calls[0].function.arguments)
end
LLM-->>Pipeline: extracted_data, resp
Pipeline->>Pipeline: postprocess(extracted_data.model_dump())
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
Next review available in: 44 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds typed support for ChangesExtraction method support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant extract
participant ExtractionPipeline
participant create_text_completion
participant LiteLLM
extract->>ExtractionPipeline: extraction_method
ExtractionPipeline->>create_text_completion: configured extraction_method
create_text_completion->>LiteLLM: response_format or tool-call request
LiteLLM-->>create_text_completion: model response
create_text_completion-->>ExtractionPipeline: validated extraction result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- When using tool-based extraction you assume
resp.choices[0].message.tool_calls[0].function.argumentsis always present; consider adding defensive checks (or a model-capability fallback toresponse_format) to avoid crashes on models that don’t return tool calls. - The hardcoded
reasoning_effort = 'none'for all tool-calling requests is described as needed only for gpt-5.6 models; you might want to gate this bymodel_nameor make it configurable so other models can still use their default reasoning settings.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When using tool-based extraction you assume `resp.choices[0].message.tool_calls[0].function.arguments` is always present; consider adding defensive checks (or a model-capability fallback to `response_format`) to avoid crashes on models that don’t return tool calls.
- The hardcoded `reasoning_effort = 'none'` for all tool-calling requests is described as needed only for gpt-5.6 models; you might want to gate this by `model_name` or make it configurable so other models can still use their default reasoning settings.
## Individual Comments
### Comment 1
<location path="src/perla_extract/llm_call.py" line_range="67-69" />
<code_context>
"""Custom exception to indicate that the maximum number of retries has been exceeded."""
pass
+def format_schema(model: BaseModel) -> dict:
+ #adaptede from https://github.com/567-labs/instructor/blob/47fdb2ca07119d389a3c0e8bc28b9930b814f294/instructor/v2/providers/openai/schema.py
+ schema = model.model_json_schema()
+ parameters = {k: v for k, v in schema.items() if k not in ("title", "description")}
</code_context>
<issue_to_address>
**nitpick (typo):** Fix minor typo in the comment referencing the source implementation.
In the inline comment, change `adaptede` to `adapted` to correct the typo in the attribution.
```suggestion
def format_schema(model: BaseModel) -> dict:
#adapted from https://github.com/567-labs/instructor/blob/47fdb2ca07119d389a3c0e8bc28b9930b814f294/instructor/v2/providers/openai/schema.py
schema = model.model_json_schema()
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/perla_extract/pipeline.py (1)
323-338: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winForward extraction configuration in the Nomad path.
extract_from_pdf_nomadcallscreate_text_completionat lines 348-350 withoutself.extraction_methodorself.additional_params. A Nomad extraction configured for"response_format"silently uses the"tool_call"default.Proposed fix
results, completion_usage = llm_call.create_text_completion( - self.model_name, pdf_text, api_key=api_key, api_base_url=api_base_url + self.model_name, + pdf_text, + api_key=api_key, + api_base_url=api_base_url, + extraction_method=self.extraction_method, + additional_params=self.additional_params, )🤖 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/perla_extract/pipeline.py` around lines 323 - 338, Update the Nomad extraction flow in extract_from_pdf_nomad so its create_text_completion call forwards self.extraction_method and self.additional_params. Preserve the configured values, including response_format, instead of allowing create_text_completion to fall back to the tool_call default.
🤖 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/perla_extract/llm_call.py`:
- Line 219: Update the tool-call request parameter handling around
filtered_params to use setdefault for reasoning_effort, retaining "none" as the
default while preserving any explicit value supplied through additional_params.
- Around line 233-237: Update the non-response_format branch in the LLM
extraction flow to safely handle missing or empty message.tool_calls before
indexing the first call or accessing its function arguments. Normalize absent
tool-call data into the exception type handled by the existing validation retry
logic, while preserving normal argument extraction for valid tool-call
responses.
In `@src/perla_extract/pipeline.py`:
- Around line 323-324: Update the affected constructor signature so existing
positional callers still bind their eighth argument to additional_params: place
extraction_method after additional_params, or make extraction_method
keyword-only. Preserve the current defaults and support the repository’s
existing named-argument usage.
---
Outside diff comments:
In `@src/perla_extract/pipeline.py`:
- Around line 323-338: Update the Nomad extraction flow in
extract_from_pdf_nomad so its create_text_completion call forwards
self.extraction_method and self.additional_params. Preserve the configured
values, including response_format, instead of allowing create_text_completion to
fall back to the tool_call default.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 46721b4c-7b73-44fd-bb8e-e09ae273126e
📒 Files selected for processing (4)
src/perla_extract/configuration.pysrc/perla_extract/llm_call.pysrc/perla_extract/pipeline.pysrc/perla_extract/pydantic_model_reduced.py
There was a problem hiding this comment.
Pull request overview
This PR changes the extraction pipeline to use tool-calling (function/tool schema) by default instead of response_format, while still supporting response_format as an alternative. It also propagates an extraction_method option through the pipeline and extends the reduced Pydantic schema with a unit field on AdditionalParameter.
Changes:
- Add
EXTRACTION_METHODSand thread a newextraction_methodoption throughpipeline.py→llm_call.py. - Implement tool-calling schema formatting + forced tool selection for structured extraction.
- Extend
AdditionalParameterwith aunitfield.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/perla_extract/pydantic_model_reduced.py |
Adds AdditionalParameter.unit and adjusts nearby schema/comment formatting. |
src/perla_extract/pipeline.py |
Propagates extraction_method into LLM calls and simplifies failure raw-output capture. |
src/perla_extract/llm_call.py |
Adds tool-calling path (schema formatting + tool choice) and routes validation to tool-call arguments. |
src/perla_extract/configuration.py |
Introduces EXTRACTION_METHODS Literal for consistent method typing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| { | ||
| "role": "user", | ||
| "content": f"{system_prompt}\n{instruction}\n Here is the schema: {PerovskiteSolarCells.model_json_schema()!s} \n\nHere is the text:\n{pdf_text}", | ||
| "content": f"{system_prompt}\n{instruction} \n\nHere is the text:\n{pdf_text}", |
There was a problem hiding this comment.
did you ablate that it is generally better not to show the schema?
I would think so, but did you test?
There was a problem hiding this comment.
The evals that i ran last week were with the schema removed from the prompt. I would say still not conclusive.
kjappelbaum
left a comment
kjappelbaum
left a comment
There was a problem hiding this comment.
makes generally sense to me. I only had one question (also for understanding how this would impact our old results)
Added option to use tool calling (now default) to extract data instead of the response format option.
tested with:
reasoning effort needs to be explicitly set to 'none' for the gpt-5.6 models so by default am setting it to 'none' when using tool_calling
with
openrouter/gpt-oss-120bit fails as the response contains only the reasoning content and nothing in the message.content or message.tool_callsSummary by Sourcery
Switch the extraction flow to use tool-based schema calling by default while keeping response_format as an optional alternative, and update related configuration and models.
New Features:
Enhancements:
Build:
Summary by CodeRabbit