Skip to content

Conversation

@soulkeykim
Copy link

@soulkeykim soulkeykim commented Sep 9, 2025

Added embeddinggemma to the embedding models list and expanded the tool models with llama3.3, qwq, and llama3.2 alongside the existing gpt-oss entries. This keeps the constants aligned with the latest Ollama-supported models and ensures broader compatibility for embedding and tool use cases.

Summary by CodeRabbit

  • New Features
    • Added support for new Ollama models.
      • Embeddings: Embedding Gemma
      • Tool-use: gpt-oss 20B and gpt-oss 120B
    • These models are now available for selection wherever model choice is supported.
    • No changes required to existing workflows.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 9, 2025

Walkthrough

Updated constants in a single file to include additional Ollama model identifiers: one embedding model and two tool-capable base models. No other logic, control flow, or files were changed.

Changes

Cohort / File(s) Summary of Changes
Ollama model constants
src/lfx/src/lfx/base/models/ollama_constants.py
Added embeddinggemma to OLLAMA_EMBEDDING_MODELS. Added gpt-oss:20b and gpt-oss:120b to OLLAMA_TOOL_MODELS_BASE. No other modifications.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Pre-merge checks (2 passed, 1 inconclusive)

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The pull request title “Update ollama_constants.py” is generic and does not clearly summarize the specific additions of new embedding and tool model constants. Although it correctly identifies the file being modified, it fails to highlight the most important changes from the developer’s perspective. A more descriptive title would help reviewers understand the key updates at a glance. Rename the title to something more specific such as “Add embeddinggemma and new tool models to ollama_constants” to better reflect the primary changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed The description accurately relates to the changeset by detailing the additions to the embedding and tool model constant lists and explains the purpose of aligning with the latest Ollama-supported models. It remains on-topic and clearly conveys the rationale and scope of the updates.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a90bece and a7a3a6c.

📒 Files selected for processing (1)
  • src/lfx/src/lfx/base/models/ollama_constants.py (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Run Ruff Check and Format
  • GitHub Check: Update Starter Projects
  • GitHub Check: Ruff Style Check (3.13)
🔇 Additional comments (3)
src/lfx/src/lfx/base/models/ollama_constants.py (3)

3-3: EmbeddingGemma addition looks correct (LGTM).

Verified slug exists and is an embedding model on Ollama. Note: the model page indicates it requires Ollama v0.11.10+. (ollama.com)


16-17: gpt-oss:20b and :120b are valid tool-capable slugs (LGTM).

Both models are listed in the Ollama library with tool-use support; launch blog was published August 5, 2025. (ollama.com)


16-17: PR description vs. diff mismatch—please reconcile.

The PR text says the tools list was expanded to include “llama3.3”, “qwq”, and “llama3.2” (these already appear in the list), while the diff shows only the addition of “gpt-oss:20b” and “gpt-oss:120b”. Please update the description or adjust the change set accordingly.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 9, 2025

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