docs/backlog.md (1)

9-11: Appropriate addition to backlog.

This backlog item for comparing the sizes of CrateDB's llms.txt against other vendors aligns well with the feature of loading context files from the local filesystem.

Consider using proper Markdown link syntax for the GitHub issue URL:

-  https://github.com/crate/about/issues/20
+  [#20](https://github.com/crate/about/issues/20)

This would address the markdownlint MD034 warning about bare URLs.

🧰 Tools

🪛 markdownlint-cli2 (0.17.2)

11-11: Bare URL used
null

(MD034, no-bare-urls)

README.md (1)

181-183: Documentation update properly reflects the new capability.

The documentation has been updated to clearly indicate that ABOUT_CONTEXT_URL can now accept both remote URLs and local filesystem paths, which aligns with the PR objective of enhancing flexibility in specifying context files.

Consider wrapping the default URL in backticks for consistency with the environment variable formatting:

-The default value is https://cdn.crate.io/about/v1/llms-full.txt.
+The default value is `https://cdn.crate.io/about/v1/llms-full.txt`.

This would also address the markdownlint MD034 warning about bare URLs.

🧰 Tools

🪛 markdownlint-cli2 (0.17.2)

183-183: Bare URL used
null

(MD034, no-bare-urls)

src/cratedb_about/query/model.py (2)

75-76: Consider path normalization for cross-platform compatibility.

The current implementation might have issues with path separators across different operating systems. Consider normalizing the path or using platform-specific path handling.

        url = cls.llms_txt_url
        path = Path(url)
+       # Normalize path for cross-platform compatibility
+       path = path.expanduser().resolve()
        if path.exists():
            return path.read_text()

---

78-78: Consider adding HTTP response validation.

When fetching content via HTTP, it's good practice to check the response status code to ensure the request was successful.

        if url.startswith("http"):
-           return requests.get(url, timeout=10).text
+           response = requests.get(url, timeout=10)
+           response.raise_for_status()  # Raises HTTPError for bad responses
+           return response.text

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

src/cratedb_about/query/model.py (1)

65-79: 🛠️ Refactor suggestion

Consider adding path validation for security.

Loading files from the filesystem without path validation could pose a security risk. Consider validating the path to prevent access to sensitive files.

    @classproperty
    def llms_txt_payload(cls) -> str:
        """
        Retrieve payload of context file.

        TODO: Add third option `pueblo.to_io`, to load resources from anywhere.
              See `cratedb_about.outline.core`.
        """
        url = cls.llms_txt_url
        path = Path(url)
+       # Validate path is within allowed directories to prevent path traversal attacks
+       allowed_dirs = [Path.cwd(), Path.home() / "cratedb_context"]
+       if path.exists() and path.is_file() and any(
+           path.resolve().is_relative_to(allowed_dir) for allowed_dir in allowed_dirs
+       ):
-       if path.exists():
            return path.read_text()
        if url.startswith("http"):
            return requests.get(url, timeout=10).text
-       raise NotImplementedError(f"Unable to load context file. Source: {url}")
+       raise NotImplementedError(f"Unable to load context file or path not allowed. Source: {url}")

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 76-76: src/cratedb_about/query/model.py#L76
Added line #L76 was not covered by tests

README.md (1)

183-183: Fix bare URL according to markdown linting rules.

The URL on line 183 is flagged by markdownlint as a bare URL (MD034). Consider wrapping it in angle brackets.

-The default value is https://cdn.crate.io/about/v1/llms-full.txt.
+The default value is <https://cdn.crate.io/about/v1/llms-full.txt>.

🧰 Tools

🪛 markdownlint-cli2 (0.17.2)

183-183: Bare URL used
null

(MD034, no-bare-urls)

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

src/cratedb_about/query/model.py (2)

70-71: Consider tracking the TODO separately.

The TODO comment about adding a third option with pueblo.to_io might be better tracked in an issue system rather than in code comments, especially if it represents planned future work.

---

76-78: Add a simple try-except for file reading.

While your preference for generic exception handling is noted (and respected in the get_prompt method), consider adding a try-except block specifically around the path.read_text() call to catch file reading errors at their source.

if path.exists():
-    return path.read_text()
+    try:
+        return path.read_text()
+    except Exception as e:
+        raise Exception(f"Error reading file {path}: {e}")

tests/test_query.py (1)

104-106: Remove redundant comment.

There are two comments explaining the same thing - one at line 105 is redundant with line 104. Consider removing one of them for clarity.

    # Set the environment variable to the HTTP URL.
-   # Point `ABOUT_CONTEXT_URL` to the temporary URL.
    monkeypatch.setenv("ABOUT_CONTEXT_URL", test_url)

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

Learnt from: amotl
PR: crate/about#29
File: src/cratedb_about/query/model.py:75-76
Timestamp: 2025-05-15T11:27:34.514Z
Learning: For the `cratedb_about` repository, the maintainer prefers generic exception handling rather than catching specific exception types.