Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix KeyError when specifying function_calling_llm in agents.yaml #2331

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

devin-ai-integration[bot]
Copy link
Contributor

Fixes #2330 - This PR addresses the KeyError that occurs when specifying function_calling_llm as a string in agents.yaml. The fix adds a try/except block similar to how llm is handled, allowing users to specify external LLM names like 'gpt-4o-mini' directly in the YAML configuration.

Link to Devin run: https://app.devin.ai/sessions/96c5b3b0122a4ef097edea05b07cf109
Requested by: Human

Co-Authored-By: Joe Moura <joao@crewai.com>
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add "(aside)" to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Sorry, something went wrong.

@joaomdmoura
Copy link
Collaborator

Disclaimer: This review was made by a crew of AI Agents.

Code Review Comment for PR #2331

Overview

This PR addresses a KeyError issue when specifying function_calling_llm in the agents.yaml configuration. The changes enhance error handling and introduce new test coverage, which contributes to the overall robustness of the codebase.

Changes Summary

  1. File: src/crewai/project/crew_base.py

    • Implementation of a try-except block for better error handling when accessing function_calling_llm.
    • Added flexibility for specifying LLMs directly or via agent references.
  2. File: tests/yaml_config_test.py

    • Comprehensive test coverage to validate the new configurations and error handling.

Code Quality Findings

  • Positive Aspects:

    • The new error handling approach significantly improves robustness.
    • Backward compatibility is preserved by allowing both direct model specification and agent reference.
    • Tests are well-structured, providing clear intent and coverage.
  • Issues and Recommendations:

    1. Error Handling Specificity:
    except KeyError as e:
        logger.debug(f"No agent found for function_calling_llm '{function_calling_llm}', using it as direct model name")
        self.agents_config[agent_name]["function_calling_llm"] = function_calling_llm
    • Justification: Adding context to the error handling will assist debugging efforts.
    1. Type Validation:
    if function_calling_llm := agent_info.get("function_calling_llm"):
        if not isinstance(function_calling_llm, str):
            raise ValueError(f"function_calling_llm must be a string, got {type(function_calling_llm)}")
    • Justification: Validating types upfront can catch configuration issues early, improving stability.

Test Enhancements

  • Missing Edge Cases:

    • Consider adding tests for invalid types and empty configurations.
    def test_invalid_function_calling_llm_type():
        with pytest.raises(ValueError):
            agents_config = {"test_agent": {"function_calling_llm": 123}}
  • Documentation within Tests:

"""
Test function_calling_llm YAML configuration.

Tests:
    - Direct model name specification
    - Agent reference resolution
    - Configuration persistence
    - Integration with Agent initialization
"""
  • Justification: Clear documentation helps future contributors understand test intent.

General Recommendations

  1. Documentation Updates:

    • Ensure that the docstring for function_calling_llm reflects its dual functionality for clarity.
    • Update the README to include these new configuration options.
  2. Validation and Logging Enhancements:

    • Consider implementing constraints for the model names that can be directly specified.
    • Add debug logging to trace configuration resolution, aiding in support and debugging processes.
  3. Seek Related PRs:

    • Check for related PRs that address similar issues to unify learnings and avoid redundancy.

Conclusion

The changes effectively mitigate the KeyError issue while maintaining backward compatibility. Complementary to this, the addition of robust test coverage provides a safety net for future modifications. The main recommendations revolve around improving error handling specificity, introducing type validations, and enriching test coverage with edge case scenarios. Taking these suggestions into account will contribute to a more resilient codebase and a smoother development experience going forward.

devin-ai-integration bot and others added 2 commits March 11, 2025 08:50
Co-Authored-By: Joe Moura <joao@crewai.com>
Co-Authored-By: Joe Moura <joao@crewai.com>
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.

[BUG]Keyerror when specifying function_calling_llm in agents.yaml
1 participant