Repository navigation
ROB-1767 use correct date - #794
Merged
Merged
Conversation
Contributor
WalkthroughThis update enhances the prompt rendering system by adding the current year to the context available in Jinja2 templates. It updates an existing template to clarify how to interpret dates lacking explicit years and introduces new test fixtures and cases to verify log and event retrieval based on date disambiguation, particularly for scenarios involving year inference. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PromptEngine
participant Jinja2Template
participant SystemClock
User->>PromptEngine: Submit prompt with date (possibly without year)
PromptEngine->>SystemClock: Get current datetime
SystemClock-->>PromptEngine: Return current datetime
PromptEngine->>Jinja2Template: Render template with context (now, now_timestamp_seconds, current_year)
Jinja2Template-->>PromptEngine: Rendered prompt with date disambiguation
PromptEngine-->>User: Return processed prompt/response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ 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)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txt (1)
1-9: Same date/window mismatch as previous fixtureThis “default” log snapshot still embeds 2025-06 timestamps while the file name &
match_paramsclaim Dec 2024. Please reconcile the data or adjust the naming/parameters.tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_Error_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txt (1)
1-9: Capital-“Error” variant inherits the same inconsistenciesThe issues called out for the lowercase-error fixture (wrong year, mismatching JSON params, inclusion of non-error severities) apply here as well.
🧹 Nitpick comments (6)
holmes/plugins/prompts/__init__.py (2)
24-24: Use a context-manager when opening files
open(path, …).read()leaves the file descriptor open until garbage collection.
Wrap it in awithblock to close immediately and avoid FD leaks (especially relevant when many prompts are rendered in long-lived processes).- return open(path, encoding="utf-8").read() + with open(path, encoding="utf-8") as f: + return f.read()
46-52: Do not silently overwrite caller-supplied context keys
dict.update()will override keys provided by the caller (e.g. a custom"current_year"for back-dated prompts).
Protect user intent withsetdefaultor a new dict instead of in-place mutation:- context.update( - { - "now": f"{now}", - "now_timestamp_seconds": int(now.timestamp()), - "current_year": now.year, - } - ) + safe_defaults = { + "now": f"{now}", + "now_timestamp_seconds": int(now.timestamp()), + "current_year": now.year, + } + for k, v in safe_defaults.items(): + context.setdefault(k, v)holmes/plugins/prompts/_current_date_time.jinja2 (1)
2-2: Minor wording tweakThe word “either” is left hanging without a matching “or”.
-When users mention dates without years (e.g., 'March 25th', 'last May', etc.), assume they either mean the current year ({{ current_year }}) unless context suggests otherwise. +When users mention dates without years (e.g., 'March 25th', 'last May', etc.), assume they mean the current year ({{ current_year }}) unless context suggests otherwise.tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/fetch_configuration_changes2025-06-12T23_59_59Z_2025-06-12T00_00_00Z.txt (1)
1-10: Filename ‑vs-content date order is reversedThe filename encodes
end_datetime→start_datetime, while the JSON in Line 1 uses the conventionalstart_datetimefirst. This inversion makes it harder to locate fixtures by lexical ordering and invites future confusion.Consider renaming the file (or swapping the two ISO strings) so that both filename and payload list the start date first.
tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/get_current_time_.txt (1)
1-8: Trailing “_” in filename looks accidental
get_current_time_.txtends with an underscore before the extension. Unless this is intentional (e.g., to avoid a clash), dropping the underscore will keep naming consistent with other fixtures.tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/test_case.yaml (1)
6-9: Over-prescriptive assertions risk brittle testsListing the full log line strings (including exact wording and date prefix) forces the LLM to echo them verbatim. Per past guidance, use descriptive assertions (e.g., regexes or contains-clauses) so alternative yet correct phrasings don’t fail the test.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
holmes/plugins/prompts/__init__.py(1 hunks)holmes/plugins/prompts/_current_date_time.jinja2(1 hunks)tests/llm/fixtures/test_ask_holmes/50_logs_since_specific_date/test_case.yaml(0 hunks)tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_Error_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txt(1 hunks)tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txt(1 hunks)tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_error_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txt(1 hunks)tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs_default.txt(1 hunks)tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/get_current_time_.txt(1 hunks)tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/kubectl_find_resource_robusta-holmes_pod.txt(1 hunks)tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/test_case.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/toolsets.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/fetch_configuration_changes2025-06-12T23_59_59Z_2025-06-12T00_00_00Z.txt(1 hunks)tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/get_current_time_.txt(1 hunks)tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/test_case.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/toolsets.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- tests/llm/fixtures/test_ask_holmes/50_logs_since_specific_date/test_case.yaml
🧰 Additional context used
📓 Path-based instructions (3)
holmes/plugins/prompts/**/*.jinja2
📄 CodeRabbit Inference Engine (CLAUDE.md)
Prompts must be located in holmes/plugins/prompts/{name}.jinja2
Files:
holmes/plugins/prompts/_current_date_time.jinja2
tests/llm/fixtures/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Mock data must be located in tests/llm/fixtures/{test_name}/
Files:
tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/toolsets.yamltests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/fetch_configuration_changes2025-06-12T23_59_59Z_2025-06-12T00_00_00Z.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/toolsets.yamltests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/get_current_time_.txttests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/test_case.yamltests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/get_current_time_.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/test_case.yamltests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/kubectl_find_resource_robusta-holmes_pod.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs_default.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_error_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_Error_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txt
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: ALWAYS place Python imports at the top of the file, not inside functions or methods
Use Ruff for formatting and linting (configured in pyproject.toml)
Type hints required (mypy configuration in pyproject.toml)
Files:
holmes/plugins/prompts/__init__.py
🧠 Learnings (11)
📓 Common learnings
Learnt from: nherment
PR: robusta-dev/holmesgpt#436
File: tests/llm/fixtures/test_ask_holmes/42_dns_issues_result_new_tools/toolsets.yaml:6-20
Timestamp: 2025-06-05T06:14:11.571Z
Learning: nherment prefers to keep test fixtures simple rather than adding complex security restrictions, even when potential security issues are identified.
📚 Learning: applies to holmes/plugins/prompts/**/*.jinja2 : prompts must be located in holmes/plugins/prompts/{n...
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-06T08:36:24.917Z
Learning: Applies to holmes/plugins/prompts/**/*.jinja2 : Prompts must be located in holmes/plugins/prompts/{name}.jinja2
Applied to files:
holmes/plugins/prompts/_current_date_time.jinja2holmes/plugins/prompts/__init__.py
📚 Learning: applies to holmes/plugins/toolsets/**/*.yaml : toolsets must be located in holmes/plugins/toolsets/{...
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-06T08:36:24.917Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Toolsets must be located in holmes/plugins/toolsets/{name}.yaml or {name}/
Applied to files:
tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/toolsets.yamltests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/toolsets.yaml
📚 Learning: in robusta-dev/holmesgpt config.example.yaml, the azuremonitorlogs toolset configuration shows "enab...
Learnt from: vishiy
PR: robusta-dev/holmesgpt#782
File: config.example.yaml:31-49
Timestamp: 2025-08-05T00:42:23.792Z
Learning: In robusta-dev/holmesgpt config.example.yaml, the azuremonitorlogs toolset configuration shows "enabled: true" as an example of how to enable the toolset, not as a default setting. The toolset is disabled by default and requires explicit enablement in user configurations.
Applied to files:
tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/toolsets.yamltests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/toolsets.yamltests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/kubectl_find_resource_robusta-holmes_pod.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_error_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_Error_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txt
📚 Learning: new toolsets require integration tests with mocks...
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-06T08:36:24.917Z
Learning: New toolsets require integration tests with mocks
Applied to files:
tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/toolsets.yamltests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/toolsets.yamltests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/get_current_time_.txttests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/get_current_time_.txt
📚 Learning: in the kubernetes logs toolset for holmes, both current and previous logs are intentionally fetched ...
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
Applied to files:
tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/toolsets.yamltests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/toolsets.yamltests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/kubectl_find_resource_robusta-holmes_pod.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs_default.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_error_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_Error_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txt
📚 Learning: in llm-as-judge test cases for holmesgpt, expected outputs should be descriptive rather than prescri...
Learnt from: Sheeproid
PR: robusta-dev/holmesgpt#586
File: tests/llm/fixtures/test_ask_holmes/03_what_is_the_command_to_port_forward/test_case.yaml:4-4
Timestamp: 2025-07-02T10:27:17.231Z
Learning: In LLM-as-judge test cases for HolmesGPT, expected outputs should be descriptive rather than prescriptive when testing for flexible responses like port numbers. Using specific values in expected outputs can cause unnecessary test failures when the AI generates different but equally valid responses.
Applied to files:
tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/test_case.yamltests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/test_case.yamltests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/kubectl_find_resource_robusta-holmes_pod.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs_default.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_error_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txttests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/fetch_pod_logs2024-12-31T23_59_59Z_Error_default_robusta-holmes-6bbcd9f6f5-2q4jc_2024-12-01T00_00_00Z.txt
📚 Learning: applies to tests/llm/fixtures/** : mock data must be located in tests/llm/fixtures/{test_name}/...
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-06T08:36:24.917Z
Learning: Applies to tests/llm/fixtures/** : Mock data must be located in tests/llm/fixtures/{test_name}/
Applied to files:
tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/test_case.yaml
📚 Learning: all new features require unit tests...
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-06T08:36:24.917Z
Learning: All new features require unit tests
Applied to files:
tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/test_case.yaml
📚 Learning: resource naming in evals: never use names that hint at the problem or expected behavior (e.g., avoid...
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-06T08:36:24.917Z
Learning: Resource naming in evals: Never use names that hint at the problem or expected behavior (e.g., avoid broken-pod, test-project-that-does-not-exist, crashloop-app). Use neutral names that don't give away what the LLM should discover
Applied to files:
tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/test_case.yamltests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/kubectl_find_resource_robusta-holmes_pod.txt
📚 Learning: for evaluation test fixtures in the holmesgpt project, security contexts and security hardening are ...
Learnt from: aantn
PR: robusta-dev/holmesgpt#783
File: tests/llm/fixtures/test_ask_holmes/100_historical_logs/payment-api.yaml:49-70
Timestamp: 2025-08-05T06:14:39.523Z
Learning: For evaluation test fixtures in the holmesgpt project, security contexts and security hardening are not priorities. The focus should be on functionality and test reliability rather than adding security configurations to Kubernetes manifests used in evals.
Applied to files:
tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/kubectl_find_resource_robusta-holmes_pod.txt
⏰ 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). (2)
- GitHub Check: llm_evals
- GitHub Check: Pre-commit checks
🔇 Additional comments (6)
tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/toolsets.yaml (1)
1-5: Fixture configuration looks goodToolsets are enabled explicitly and the file sits under the correct fixture path.
No issues spotted.tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/toolsets.yaml (1)
1-5: Fixture configuration looks goodSame comment as above – everything is in the expected place and format.
tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/get_current_time_.txt (1)
1-7: Static time fixture acceptedThe captured output is well-formed and follows existing fixture style.
Just remember to refresh if the reference “now” ever changes in related tests.tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/fetch_configuration_changes2025-06-12T23_59_59Z_2025-06-12T00_00_00Z.txt (1)
1-10: Double-nesting may violate fixture-location guidelineOrg guideline: “Mock data must be located in
tests/llm/fixtures/{test_name}/”.This file sits under
tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/…,
adding an extra level (93_events_since_specific_date).
Please confirm that the additional sub-directory is acceptable for the test harness; if not, flatten it one level up.tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/kubectl_find_resource_robusta-holmes_pod.txt (1)
1-3: Confirm that embedded resource name doesn’t leak intentA learning notes that fixture resource names should not hint at the underlying issue (e.g.,
broken-pod).
robusta-holmesappears benign, but double-check that it’s not steering the LLM toward a specific answer.tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/test_case.yaml (1)
1-11: LGTM – test case is concise and uses descriptive expected outputNo issues spotted. Matches the best-practice of descriptive, not prescriptive, assertions.
aantn
approved these changes
Aug 12, 2025
aantn
enabled auto-merge (squash)
August 12, 2025 07:21
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.