examples/ask.py (3)

36-38: Add type hints to improve code quality and maintainability.

Adding type hints would improve code clarity and enable static type checking, which could help catch issues early.

-    backend: t.Literal["claude", "openai"] = "openai"
-    use_knowledge: bool = True
+    backend: t.Literal["claude", "openai"] = "openai"
+    use_knowledge: bool = True
+
+    def ask(self, question: str) -> None:

This relates to the commented-out mypy in pyproject.toml - adding proper type hints would allow you to re-enable type checking.

---

122-140: Make backend configurable and add error handling in main.

The main function has hardcoded values and minimal error handling.

Consider enhancing the main function to:

1. Accept backend configuration from environment variables or command line
2. Add better error handling
3. Make the interface more user-friendly

 def main():
     """
     Ready.
     """
+    # Get backend from environment variable or use default
+    backend = os.environ.get("CRATEDB_LLM_BACKEND", "openai").lower()
+    if backend not in ["openai", "claude"]:
+        print(f"Warning: Unknown backend '{backend}'. Using 'openai' instead.", file=sys.stderr)  # noqa: T201
+        backend = "openai"
+        
     wizard = CrateDBConversation(
-        backend="openai",
+        backend=backend,
         use_knowledge=True,
     )
     if len(sys.argv) >= 2:
         question = sys.argv[1]
         if question == "list-questions":
             print("A few example questions:\n")  # noqa: T201
-            print("\n\n".join(Example.questions))  # noqa: T201
+            for i, q in enumerate(Example.questions, 1):
+                print(f"{i}. {q}")  # noqa: T201
             return
+        elif question in ["-h", "--help"]:
+            print("Usage: python ask.py [QUESTION|list-questions|--help]")  # noqa: T201
+            print("\nExamples:")  # noqa: T201
+            print("  python ask.py 'What are the benefits of CrateDB?'")  # noqa: T201
+            print("  python ask.py list-questions")  # noqa: T201
+            print("\nEnvironment variables:")  # noqa: T201
+            print("  OPENAI_API_KEY - Required when using the OpenAI backend")  # noqa: T201
+            print("  CRATEDB_LLM_BACKEND - Set to 'openai' or 'claude' (default: openai)")  # noqa: T201
+            return
     else:
         question = Example.questions[4]
     print(f"Question: {question}\nAnswer:")  # noqa: T201
-    wizard.ask(question)
+    try:
+        wizard.ask(question)
+    except Exception as e:
+        print(f"Error: {e}", file=sys.stderr)  # noqa: T201
+        sys.exit(1)

---

1-11: Enhance script dependency declaration.

The script uses a comment-based format for dependencies, but it could be improved.

Consider converting to a more standardized format like pyproject.toml for the script, or add a requirements.txt file. This would make it easier for users to install dependencies:

# requirements.txt
claudette>=0.1.0
openai>=1.0.0
requests>=2.0.0

Or update the comment to better explain how to install these dependencies manually:

 # /// script
 # requires-python = ">=3.8"
 # dependencies = [
 #     "claudette",
 #     "openai",
 #     "requests",
 # ]
 # ///
+# To install dependencies: pip install claudette openai requests

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

docs/backlog.md (1)

6-10: Improve Markdown formatting for URL references.

The Markdown linter identified bare URLs in the document. Consider using proper Markdown link syntax for better readability and maintainability.

-## Iteration +2
-- Unlock Discourse: https://community.cratedb.com/raw/1015
-- Unlock HTML resources: https://www.urltoany.com/url-to-markdown.
-  => Find the best standalone program.
-- Unlock GitHub projects: https://github.com/mattduck/gh2md
+## Iteration +2
+- Unlock Discourse: [Discourse Raw Content](https://community.cratedb.com/raw/1015)
+- Unlock HTML resources: [URL to Markdown Converter](https://www.urltoany.com/url-to-markdown).
+  => Find the best standalone program.
+- Unlock GitHub projects: [gh2md GitHub Repository](https://github.com/mattduck/gh2md)

🧰 Tools

🪛 markdownlint-cli2 (0.17.2)

7-7: Bare URL used
null

(MD034, no-bare-urls)

---

8-8: Bare URL used
null

(MD034, no-bare-urls)

---

10-10: Bare URL used
null

(MD034, no-bare-urls)

src/cratedb_about/cli.py (1)

31-33: Use Click's echo functions instead of direct stdout writes.

Click provides click.echo() which handles Unicode and different output streams better than direct writes to stdout.

-    sys.stdout.write(f"Question: {question}\nAnswer:\n")
-    sys.stdout.write(wizard.ask(question))
-    sys.stdout.write("\n")
+    click.echo(f"Question: {question}")
+    click.echo("Answer:")
+    click.echo(wizard.ask(question))

.github/workflows/tests.yml (1)

1-65: Well-structured GitHub Actions workflow for automated testing

The workflow is well-configured with appropriate triggers for push events, pull requests, and manual dispatch. The job matrix strategy with multiple Python versions is a good practice for ensuring compatibility.

A few suggestions for enhancement:

1. Consider adding a job timeout to prevent runaway workflows
2. Add a step to verify the installation by checking the CLI version
3. Consider creating a separate job for linting to provide faster feedback

# Add timeout to prevent runaway workflows
 jobs:

   test:
+    timeout-minutes: 15
     name: "
     Python ${{ matrix.python-version }}
     "

# Add version check after installation
     - name: Set up project
       run: |
         uv pip install --editable='.[develop,test]'
+        cratedb-about --version

src/cratedb_about/core.py (4)

1-2: Update or remove stale comment reference

The comment appears to be a reference to source inspiration, but it would be more valuable to provide context about what specific patterns or ideas were derived from the referenced URL.

Either expand on what was derived from the referenced website or consider removing if not directly relevant to understanding the code.

---

18-23: Method implementation looks good but could be more robust

The ask method effectively routes questions to the appropriate backend implementation.

Consider adding error handling to catch and provide helpful messages for potential API failures:

def ask(self, question: str) -> str:
    try:
        if self.backend == "openai":
            return self.ask_gpt(question)
        if self.backend == "claude":
            return self.ask_claude(question)
        raise NotImplementedError("Please select an available LLM backend")
+    except Exception as e:
+        return f"Error querying LLM backend: {str(e)}"

---

52-53: Optimize imports for better code organization

The import for Message is placed after instantiating the OpenAI client, which disrupts the logical flow of imports.

Move the import to be grouped with the other OpenAI imports:

from openai import OpenAI
from openai.types.responses import ResponseInputTextParam
from openai.types.shared_params import Reasoning
+from openai.types.responses.response_input_param import Message

client = OpenAI(api_key=os.environ.get("OPENAI_API_KEY"))
-from openai.types.responses.response_input_param import Message

---

56-57: Document model selection rationale

The code comments out "gpt-4o" and uses "o4-mini" without explaining the rationale or performance/cost tradeoffs.

Add a comment explaining the model selection reasoning:

- # model="gpt-4o",  # noqa: ERA001
- model="o4-mini",
+ # Using o4-mini as it provides a good balance of performance and cost
+ # Consider upgrading to gpt-4o for more complex queries if needed
+ model="o4-mini",

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/cratedb_about/core.py (1)

26-34: 🛠️ Refactor suggestion

Add error handling and respect use_knowledge flag in ask_claude method

The Claude implementation has several issues that need to be addressed:

1. It doesn't respect the use_knowledge flag that controls whether to use CrateDB context
2. It lacks error handling for API failures
3. The hardcoded model index issue is acknowledged with a FIXME comment

def ask_claude(self, question: str) -> str:
    from claudette import Chat, contents, models

    # FIXME: The API does not provide name-based lookup.
    model = models[1]  # Sonnet 3.5
    chat = Chat(model, sp=Settings.instructions)
-    chat(Settings.get_prompt())
+    if self.use_knowledge:
+        try:
+            prompt = Settings.get_prompt()
+            if prompt:
+                chat(prompt)
+        except Exception as e:
+            print(f"Warning: Failed to load knowledge context: {e}", file=sys.stderr)  # noqa: T201
+    
+    try:
        result = chat(question)
        return contents(result)
+    except Exception as e:
+        raise RuntimeError(f"Claude API error: {str(e)}") from e

src/cratedb_about/core.py (2)

94-94: Fix the type issue instead of using type ignore

The type ignore suggests there's a type compatibility issue that should be fixed properly.

-            input=input_messages,  # type: ignore[arg-type]
+            input=input_messages,

Consider checking the OpenAI API documentation to ensure the input_messages variable matches the expected type for the input parameter. If necessary, adjust the type annotation on line 60 to match what the API expects.

---

1-8: Add explicit attribution in file header comment

The header comment mentions the code is derived from a source, but it would be clearer to include more details about the license or permission.

-# Derived from: https://llmstxt.org/domains.html
+# Derived from: https://llmstxt.org/domains.html
+# Used with permission under MIT License (or specify the actual license)

This adds clarity regarding the legal usage of the derived code.

src/cratedb_about/core.py (1)

10-18: 🛠️ Refactor suggestion

Add validation for API keys based on the selected backend

The class should validate the required environment variables during initialization to fail fast and provide clear error messages to users.

@dataclasses.dataclass
class CrateDBConversation:
    """
    Manage conversations about CrateDB.
+
+    Requires:
+      - OpenAI backend: Set OPENAI_API_KEY environment variable
+      - Claude backend: Set ANTHROPIC_API_KEY environment variable
    """

    backend: t.Literal["claude", "openai"] = "openai"
    use_knowledge: bool = True
+
+    def __post_init__(self):
+        """Validate configuration on initialization."""
+        if self.backend == "openai" and not os.environ.get("OPENAI_API_KEY"):
+            raise ValueError("OPENAI_API_KEY environment variable is required when using 'openai' backend")
+        elif self.backend == "claude" and not os.environ.get("ANTHROPIC_API_KEY"):
+            raise ValueError("ANTHROPIC_API_KEY environment variable is required when using 'claude' backend")

This validation aligns with the docstring in the ask function in cli.py which mentions these requirements.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

@dataclasses.dataclass
class CrateDBConversation:
    """
    Manage conversations about CrateDB.

    Requires:
      - OpenAI backend: Set OPENAI_API_KEY environment variable
      - Claude backend: Set ANTHROPIC_API_KEY environment variable
    """

    backend: t.Literal["claude", "openai"] = "openai"
    use_knowledge: bool = True

    def __post_init__(self):
        """Validate configuration on initialization."""
        if self.backend == "openai" and not os.environ.get("OPENAI_API_KEY"):
            raise ValueError("OPENAI_API_KEY environment variable is required when using 'openai' backend")
        elif self.backend == "claude" and not os.environ.get("ANTHROPIC_API_KEY"):
            raise ValueError("ANTHROPIC_API_KEY environment variable is required when using 'claude' backend")

src/cratedb_about/core.py (1)

26-40: 🛠️ Refactor suggestion

Add docstring and improve Claude implementation

The ask_claude method lacks a docstring, and uses a hardcoded model index which is brittle.

    def ask_claude(self, question: str) -> str:
+        """
+        Ask a question using Anthropic's Claude model.
+        
+        Requires the ANTHROPIC_API_KEY environment variable to be set.
+        Uses Claude Sonnet 3.5 model.
+        
+        Args:
+            question: The question to ask about CrateDB
+            
+        Returns:
+            str: The response from Claude
+            
+        Raises:
+            RuntimeError: If there's an error communicating with the Claude API
+        """
        from claudette import Chat, contents, models

-        model = models[1]  # Sonnet 3.5
+        # Using explicit model reference with fallback for library changes
+        model_name = "claude-3-sonnet-20240229"  # Sonnet 3.5
+        try:
+            # Find model by name if available
+            model = next((m for m in models if getattr(m, "name", "") == model_name), models[1])
+        except (IndexError, TypeError):
+            # Fallback to a direct model reference if models structure changes
+            raise RuntimeError("Claude API: Unable to find suitable model")
        chat = Chat(model, sp=Settings.instructions)
        if self.use_knowledge:
            try:
                chat(Settings.get_prompt())
            except Exception as e:
                print(f"Warning: Failed to load knowledge context: {e}", file=sys.stderr)  # noqa: T201
        try:
            result = chat(question)
            return contents(result)
        except Exception as e:
            raise RuntimeError(f"Claude API error: {e}") from e

src/cratedb_about/core.py (7)

19-24: Add docstring to the ask method

The ask method lacks a docstring that explains its behavior and parameters.

    def ask(self, question: str) -> str:
+        """
+        Ask a question about CrateDB using the configured LLM backend.
+        
+        Args:
+            question: The question to ask about CrateDB
+            
+        Returns:
+            str: The response from the LLM
+            
+        Raises:
+            NotImplementedError: If an unsupported backend is specified
+            ValueError: If required environment variables are missing
+            RuntimeError: If there's an error communicating with the LLM API
+        """
        if self.backend == "openai":
            return self.ask_gpt(question)
        if self.backend == "claude":
            return self.ask_claude(question)
        raise NotImplementedError("Please select an available LLM backend")

---

66-81: Update type annotation to use built-in list

The code uses t.List which is deprecated in favor of built-in list in Python 3.9+.

-        input_messages: t.List[Message] = []
+        input_messages: list[Message] = []

---

91-101: Make OpenAI model configurable

The OpenAI model is hardcoded, which limits flexibility. Consider making it configurable.

@dataclasses.dataclass
class CrateDBConversation:
    """
    Manage conversations about CrateDB.

    Requires:
    - OPENAI_API_KEY environment variable when using "openai" backend
    - ANTHROPIC_API_KEY environment variable when using "claude" backend
    """

    backend: t.Literal["claude", "openai"] = "openai"
    use_knowledge: bool = True
+    openai_model: str = "o4-mini"

And then update the model usage:

        response = client.responses.create(
            # model="gpt-4o",  # noqa: ERA001
-            model="o4-mini",
+            model=self.openai_model,
            reasoning=Reasoning(
                effort="medium",
                # Your organization must be verified to generate reasoning summaries
                # summary="detailed",  # noqa: ERA001
            ),
            instructions=Settings.instructions,
            input=input_messages,  # type: ignore[arg-type]
        )

---

100-100: Fix type issue instead of using type ignore comment

The code uses a # type: ignore[arg-type] comment. Consider fixing the type issue properly.

-            input=input_messages,  # type: ignore[arg-type]
+            input=input_messages,

The error might be due to outdated type stubs for the OpenAI library. Consider looking into using the latest version of the library or adding proper type definitions.

---

92-98: Consider replacing commented code with configurable options

The commented code suggests alternate configurations that might be useful but are not enabled. Consider making these configurable options.

@dataclasses.dataclass
class CrateDBConversation:
    """
    Manage conversations about CrateDB.

    Requires:
    - OPENAI_API_KEY environment variable when using "openai" backend
    - ANTHROPIC_API_KEY environment variable when using "claude" backend
    """

    backend: t.Literal["claude", "openai"] = "openai"
    use_knowledge: bool = True
    openai_model: str = "o4-mini"
+    openai_reasoning_effort: t.Literal["low", "medium", "high"] = "medium"
+    openai_reasoning_summary: t.Optional[t.Literal["none", "concise", "detailed"]] = None

Then update the reasoning configuration:

        response = client.responses.create(
-            # model="gpt-4o",  # noqa: ERA001
            model=self.openai_model,
            reasoning=Reasoning(
-                effort="medium",
-                # Your organization must be verified to generate reasoning summaries
-                # summary="detailed",  # noqa: ERA001
+                effort=self.openai_reasoning_effort,
+                **({"summary": self.openai_reasoning_summary} if self.openai_reasoning_summary else {})
            ),
            instructions=Settings.instructions,
            input=input_messages,
        )

---

61-64: Use environment variable validation from __post_init__

The API key validation in ask_gpt method would be redundant if the suggested __post_init__ validation is implemented.

Once the __post_init__ validation is implemented, you could simplify this code:

-        api_key = os.environ.get("OPENAI_API_KEY")
-        if not api_key:
-            raise ValueError("OPENAI_API_KEY environment variable is required")
-        client = OpenAI(api_key=api_key)
+        client = OpenAI(api_key=os.environ.get("OPENAI_API_KEY"))

---

1-8: Move backend-specific imports to the top level

Backend-specific imports are currently inside methods. Consider moving them to the top level with conditional imports to improve performance when the class is instantiated multiple times.

# Derived from: https://llmstxt.org/domains.html
import dataclasses
import os
import sys
import typing as t

+# Import backends conditionally to avoid errors if dependencies are missing
+CLAUDE_AVAILABLE = False
+OPENAI_AVAILABLE = False
+
+try:
+    from claudette import Chat, contents, models
+    CLAUDE_AVAILABLE = True
+except ImportError:
+    pass
+
+try:
+    from openai import OpenAI
+    from openai.types.responses import ResponseInputTextParam
+    from openai.types.responses.response_input_param import Message
+    from openai.types.shared_params import Reasoning
+    OPENAI_AVAILABLE = True
+except ImportError:
+    pass
+
from cratedb_about.model import Settings

Then update the __post_init__ method to check for backend availability:

    def __post_init__(self):
        """Validate configuration."""
+        if self.backend == "openai" and not OPENAI_AVAILABLE:
+            raise ImportError("The 'openai' package is required when using the OpenAI backend")
+        elif self.backend == "claude" and not CLAUDE_AVAILABLE:
+            raise ImportError("The 'claudette' package is required when using the Claude backend")
+            
        if self.backend == "openai" and not os.environ.get("OPENAI_API_KEY"):
            raise ValueError("OPENAI_API_KEY environment variable is required when using 'openai' backend")
        elif self.backend == "claude" and not os.environ.get("ANTHROPIC_API_KEY"):
            raise ValueError("ANTHROPIC_API_KEY environment variable is required when using 'claude' backend")

And then simplify the method implementations to remove the imports:

    def ask_claude(self, question: str) -> str:
-        from claudette import Chat, contents, models

    def ask_gpt(self, question: str) -> str:
-        from openai import OpenAI
-        from openai.types.responses import ResponseInputTextParam
-        from openai.types.responses.response_input_param import Message
-        from openai.types.shared_params import Reasoning

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro