feat: Add Query class to compute QueryInterpretations from a user specified query.#152
Merged
davidlion merged 181 commits intoy-scope:mainfrom Sep 4, 2025
Merged
feat: Add Query class to compute QueryInterpretations from a user specified query.#152davidlion merged 181 commits intoy-scope:mainfrom
Query class to compute QueryInterpretations from a user specified query.#152davidlion merged 181 commits intoy-scope:mainfrom
Conversation
…ove documentation, code quality, and be archive independent.
… safely return if empty.
…o append_query_interpretation; Handle empty case in append static token; Set operator== to be default
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
tests/test-query.cpp (1)
41-46: Mirror house style: const-by-value in the forward declarationKeep const qualifiers on by-value params to match the definition and house style.
auto test_query( - string_view raw_query_string, - string_view expected_processed_query_string, + string_view const raw_query_string, + string_view const expected_processed_query_string, vector<string> const& schema_rules, set<string> const& expected_serialized_interpretations ) -> void;src/log_surgeon/wildcard_query_parser/Query.cpp (1)
163-163: Thread safety concern with global state (tracked in issue #154).The global state modification
NonTerminal::m_next_children_start = 0is not thread-safe. This is a known systemic issue being tracked in issue #154 for comprehensive resolution in a dedicated PR.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/log_surgeon/wildcard_query_parser/ExpressionView.cpp(2 hunks)src/log_surgeon/wildcard_query_parser/ExpressionView.hpp(2 hunks)src/log_surgeon/wildcard_query_parser/Query.cpp(1 hunks)tests/test-query.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,h,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.cppsrc/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
🧠 Learnings (22)
📓 Common learnings
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
📚 Learning: 2025-08-08T10:00:20.963Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:00:20.963Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp (C++), do not default the comparison operators: the class stores std::vector<std::variant<StaticQueryToken, VariableQueryToken>>, which yields only a weak ordering. A custom operator<=> that maps the variant’s weak ordering to std::strong_ordering is required; avoid suggesting defaulting there in future reviews.
Applied to files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.cppsrc/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-08T10:17:43.495Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:17:43.495Z
Learning: In src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp/.cpp (C++), do not suggest defaulting operator<=>. The project prefers a custom out-of-line comparator that explicitly handles the bool member (via explicit cast) to avoid implicit conversions; keep the current manual implementation.
Applied to files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.cppsrc/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-08T10:21:56.571Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:57-77
Timestamp: 2025-08-08T10:21:56.571Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp/.cpp (C++), for QueryInterpretation::append_query_interpretation, prefer a single overload taking QueryInterpretation const&; do not suggest adding an rvalue/move overload in future reviews for this method.
Applied to files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.cppsrc/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-08T10:22:26.739Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:98-120
Timestamp: 2025-08-08T10:22:26.739Z
Learning: In C++ file src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp, for QueryInterpretation::serialize(), prefer explicit std::holds_alternative/std::get branching over std::visit for readability; do not suggest refactoring to std::visit in future reviews.
Applied to files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.cppsrc/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-08T10:23:06.281Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:80-88
Timestamp: 2025-08-08T10:23:06.281Z
Learning: In y-scope/log-surgeon (C++), small function definitions are allowed to remain inline in headers. For src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp, do not suggest moving small methods like QueryInterpretation::append_variable_token to the .cpp for “consistency” in future reviews.
Applied to files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.cppsrc/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-13T12:05:00.245Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:05:00.245Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the generate_regex_string() method should not enforce well-formedness checks via assertions. The is_well_formed() method is intended to be used by callers at their discretion, allowing flexibility to generate regex strings from malformed views if desired.
Applied to files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.cppsrc/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-26T10:13:00.368Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: tests/test-query.cpp:61-67
Timestamp: 2025-08-26T10:13:00.368Z
Learning: In Catch2 unit tests, the REQUIRE macro already provides detailed debugging output when container comparisons (like std::set equality) fail, showing both expected and actual values. Additional CAPTURE statements are typically unnecessary for such comparisons.
Applied to files:
tests/test-query.cpp
📚 Learning: 2025-08-13T12:03:18.145Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:32-37
Timestamp: 2025-08-13T12:03:18.145Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the extend_to_adjacent_greedy_wildcards() method should only extend by one greedy wildcard character on each side, not multiple consecutive ones. This single-wildcard extension is the intended behavior for search operations.
Applied to files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.cppsrc/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2024-11-13T22:25:54.168Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#48
File: tests/test-tag.cpp:10-10
Timestamp: 2024-11-13T22:25:54.168Z
Learning: In the log-surgeon codebase (C++), particularly in the finite automata components involving the `Tag` class (`src/log_surgeon/finite_automata/Tag.hpp`), it's important to ensure that `Tag*` pointers in other objects cannot be `nullptr`. Test cases should focus on validating that these `Tag*` pointers are not null where they are used, and handle `nullptr` appropriately.
Applied to files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-15T00:13:46.717Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: tests/test-wildcard-expression.cpp:0-0
Timestamp: 2025-08-15T00:13:46.717Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpression.cpp, the escaping logic treats consecutive backslashes as escape-literal pairs, not as individual escape characters. For input like R"(a\*b\?c\\)", position 7 is the escape character and position 8 is the literal backslash being escaped, so only position 7 should have is_escape() return true.
Applied to files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.cppsrc/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-08T13:18:39.895Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:42-48
Timestamp: 2025-08-08T13:18:39.895Z
Learning: In y-scope/log-surgeon (C++), it is acceptable/preferred to keep `const` qualifiers on by-value function parameters to signal intent (e.g., in src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp). Do not suggest removing `const` from by-value parameters in future reviews.
Applied to files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.cppsrc/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-25T20:44:48.955Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:159-177
Timestamp: 2025-08-25T20:44:48.955Z
Learning: In the log-surgeon codebase, thread safety issues with global state like NonTerminal::m_next_children_start should be addressed comprehensively in dedicated PRs rather than fixed piecemeal in individual feature PRs. The user SharafMohamed prefers to defer such systemic architectural issues to separate PRs.
Applied to files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-15T12:07:58.626Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: tests/test-expression-view.cpp:7-7
Timestamp: 2025-08-15T12:07:58.626Z
Learning: In tests/test-expression-view.cpp, the `<catch2/catch_message.hpp>` header is required for clang-tidy to pass, even though it may not be directly used in the visible code.
Applied to files:
tests/test-query.cpp
📚 Learning: 2025-08-15T01:04:03.681Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/ExpressionView.cpp:11-11
Timestamp: 2025-08-15T01:04:03.681Z
Learning: In y-scope/log-surgeon C++ codebase, prefer explicit includes in implementation files (.cpp) for types that are directly used, even when those headers are already transitively included through the corresponding header file. This makes dependencies explicit and improves maintainability.
Applied to files:
tests/test-query.cpp
📚 Learning: 2025-08-13T12:06:36.584Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:06:36.584Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, caching SchemaParser::get_special_regex_characters() with a const& reference provides no performance benefit, indicating the method likely returns a reference to an existing container rather than performing expensive computations.
Applied to files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.cppsrc/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-18T12:05:55.905Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#151
File: tests/test-buffer-parser.cpp:0-0
Timestamp: 2025-08-18T12:05:55.905Z
Learning: In y-scope/log-surgeon (C++), prefer uniform initialization with braces {} when possible instead of assignment or parentheses initialization. For example, prefer `string const var{value};` over `string const var = value;`.
Applied to files:
tests/test-query.cpp
📚 Learning: 2024-10-24T15:54:35.193Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#42
File: src/log_surgeon/finite_automata/RegexNFA.hpp:442-456
Timestamp: 2024-10-24T15:54:35.193Z
Learning: In the C++ file `src/log_surgeon/finite_automata/RegexNFA.hpp`, for the `RegexNFA::serialize()` function, prioritize code clarity over efficiency when handling string operations.
Applied to files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-26T09:31:28.567Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:28.567Z
Learning: In src/log_surgeon/wildcard_query_parser, escaped wildcards (\* or \?) lose their wildcard nature entirely and become literal characters, while escaped delimiters retain their delimiter property in the delimiter table regardless of being escaped. This means boundary detection logic correctly treats escaped wildcards as non-wildcards but escaped delimiters as still being delimiters.
Applied to files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.cppsrc/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-26T09:31:46.313Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
Applied to files:
tests/test-query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.cppsrc/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-13T12:09:51.983Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpression.hpp:0-0
Timestamp: 2025-08-13T12:09:51.983Z
Learning: In y-scope/log-surgeon WildcardExpression/WildcardExpressionView architecture, wildcard-related metadata (like contains_wildcard flags) should be placed in WildcardExpressionView rather than WildcardExpression, since the view is responsible for regex generation and may represent subranges with different wildcard characteristics than the full expression.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-26T09:51:07.213Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:123-159
Timestamp: 2025-08-26T09:51:07.213Z
Learning: In log-surgeon's wildcard_query_parser, the ByteLexer's get_matching_variable_types method returns variable type IDs already ordered by priority (highest to lowest), so selecting the first ID gives the highest priority match.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
🧬 Code graph analysis (4)
tests/test-query.cpp (3)
src/log_surgeon/SchemaParser.hpp (1)
SchemaVarAST(49-56)src/log_surgeon/wildcard_query_parser/Query.cpp (1)
Query(36-76)src/log_surgeon/wildcard_query_parser/Query.hpp (2)
Query(16-16)lexer(72-73)
src/log_surgeon/wildcard_query_parser/ExpressionView.cpp (2)
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp (1)
delim_table(69-71)src/log_surgeon/wildcard_query_parser/ExpressionCharacter.hpp (2)
delim_table(33-33)delim_table(37-38)
src/log_surgeon/wildcard_query_parser/Query.cpp (2)
src/log_surgeon/wildcard_query_parser/Query.hpp (4)
Query(16-16)lexer(72-73)expression_view(109-112)regex_string(125-126)src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp (1)
suffix(79-79)
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp (3)
src/log_surgeon/wildcard_query_parser/ExpressionCharacter.hpp (9)
nodiscard(21-21)nodiscard(23-23)nodiscard(25-27)nodiscard(29-31)nodiscard(33-35)nodiscard(37-40)nodiscard(42-42)delim_table(33-33)delim_table(37-38)src/log_surgeon/wildcard_query_parser/Expression.hpp (3)
nodiscard(22-24)nodiscard(26-26)nodiscard(28-28)src/log_surgeon/Lexer.hpp (5)
nodiscard(153-153)nodiscard(155-157)nodiscard(159-161)nodiscard(163-166)nodiscard(175-181)
⏰ 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). (8)
- GitHub Check: build (ubuntu-24.04, release)
- GitHub Check: build (macos-15, debug)
- GitHub Check: lint (macos-15)
- GitHub Check: build (ubuntu-22.04, debug)
- GitHub Check: build (ubuntu-22.04, release)
- GitHub Check: build (ubuntu-24.04, debug)
- GitHub Check: build (macos-15, release)
- GitHub Check: lint (ubuntu-24.04)
🔇 Additional comments (4)
src/log_surgeon/wildcard_query_parser/Query.cpp (4)
36-76: LGTM! Query string normalization logic is correct.The constructor properly handles wildcard sequence collapsing, escape sequences, and preserves the semantic meaning of the query. The capacity reservation optimization is already in place.
148-157: Priority selection is correctly implemented.The code correctly selects the highest priority variable type by taking the first ID from the lexer-ordered list when no wildcard is present. The implementation aligns with the documented behaviour.
Based on the learning that the lexer returns variable type IDs ordered by priority (highest to lowest), this implementation correctly achieves the highest priority match.
171-174: Performance optimization opportunities noted in TODOs.The TODOs correctly identify the NFA and DFA construction as performance bottlenecks. These optimizations should be addressed when performance becomes a concern.
The code is functionally correct. The TODO comments appropriately mark areas for future optimization.
136-139: Preferfalse ==for consistency with coding guidelines.The codebase prefers
false == <expression>over!<expression>.- if (false == expression_view.is_surrounded_by_delims(lexer.get_delim_table())) { + if (false == expression_view.is_surrounded_by_delims(lexer.get_delim_table())) {Wait, this is already using the preferred style. No change needed.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/log_surgeon/wildcard_query_parser/Query.cpp (2)
83-85: Early return for empty input was already suggested.This duplicates prior feedback to move the empty-check before allocating/interpreting. No further action here if it’s tracked.
161-176: Global state reset remains a known concurrency risk (tracked elsewhere).Resetting NonTerminal::m_next_children_start without restoration is the previously discussed systemic issue; leaving as-is per decision to fix in a dedicated PR.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/log_surgeon/wildcard_query_parser/Query.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,h,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/log_surgeon/wildcard_query_parser/Query.cpp
🧠 Learnings (17)
📓 Common learnings
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
📚 Learning: 2025-08-08T10:21:56.571Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:57-77
Timestamp: 2025-08-08T10:21:56.571Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp/.cpp (C++), for QueryInterpretation::append_query_interpretation, prefer a single overload taking QueryInterpretation const&; do not suggest adding an rvalue/move overload in future reviews for this method.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-08T10:00:20.963Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:00:20.963Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp (C++), do not default the comparison operators: the class stores std::vector<std::variant<StaticQueryToken, VariableQueryToken>>, which yields only a weak ordering. A custom operator<=> that maps the variant’s weak ordering to std::strong_ordering is required; avoid suggesting defaulting there in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-08T10:23:06.281Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:80-88
Timestamp: 2025-08-08T10:23:06.281Z
Learning: In y-scope/log-surgeon (C++), small function definitions are allowed to remain inline in headers. For src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp, do not suggest moving small methods like QueryInterpretation::append_variable_token to the .cpp for “consistency” in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-08T10:17:43.495Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:17:43.495Z
Learning: In src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp/.cpp (C++), do not suggest defaulting operator<=>. The project prefers a custom out-of-line comparator that explicitly handles the bool member (via explicit cast) to avoid implicit conversions; keep the current manual implementation.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-13T12:03:18.145Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:32-37
Timestamp: 2025-08-13T12:03:18.145Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the extend_to_adjacent_greedy_wildcards() method should only extend by one greedy wildcard character on each side, not multiple consecutive ones. This single-wildcard extension is the intended behavior for search operations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-26T09:31:46.313Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-08T10:22:26.739Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:98-120
Timestamp: 2025-08-08T10:22:26.739Z
Learning: In C++ file src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp, for QueryInterpretation::serialize(), prefer explicit std::holds_alternative/std::get branching over std::visit for readability; do not suggest refactoring to std::visit in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-13T12:05:00.245Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:05:00.245Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the generate_regex_string() method should not enforce well-formedness checks via assertions. The is_well_formed() method is intended to be used by callers at their discretion, allowing flexibility to generate regex strings from malformed views if desired.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-26T09:31:28.567Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:28.567Z
Learning: In src/log_surgeon/wildcard_query_parser, escaped wildcards (\* or \?) lose their wildcard nature entirely and become literal characters, while escaped delimiters retain their delimiter property in the delimiter table regardless of being escaped. This means boundary detection logic correctly treats escaped wildcards as non-wildcards but escaped delimiters as still being delimiters.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-13T12:06:36.584Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:06:36.584Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, caching SchemaParser::get_special_regex_characters() with a const& reference provides no performance benefit, indicating the method likely returns a reference to an existing container rather than performing expensive computations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-15T00:13:46.717Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: tests/test-wildcard-expression.cpp:0-0
Timestamp: 2025-08-15T00:13:46.717Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpression.cpp, the escaping logic treats consecutive backslashes as escape-literal pairs, not as individual escape characters. For input like R"(a\*b\?c\\)", position 7 is the escape character and position 8 is the literal backslash being escaped, so only position 7 should have is_escape() return true.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-08T13:18:39.895Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:42-48
Timestamp: 2025-08-08T13:18:39.895Z
Learning: In y-scope/log-surgeon (C++), it is acceptable/preferred to keep `const` qualifiers on by-value function parameters to signal intent (e.g., in src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp). Do not suggest removing `const` from by-value parameters in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-25T20:44:48.955Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:159-177
Timestamp: 2025-08-25T20:44:48.955Z
Learning: In the log-surgeon codebase, thread safety issues with global state like NonTerminal::m_next_children_start should be addressed comprehensively in dedicated PRs rather than fixed piecemeal in individual feature PRs. The user SharafMohamed prefers to defer such systemic architectural issues to separate PRs.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2024-10-24T15:54:35.193Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#42
File: src/log_surgeon/finite_automata/RegexNFA.hpp:442-456
Timestamp: 2024-10-24T15:54:35.193Z
Learning: In the C++ file `src/log_surgeon/finite_automata/RegexNFA.hpp`, for the `RegexNFA::serialize()` function, prioritize code clarity over efficiency when handling string operations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2024-11-13T22:25:54.168Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#48
File: tests/test-tag.cpp:10-10
Timestamp: 2024-11-13T22:25:54.168Z
Learning: In the log-surgeon codebase (C++), particularly in the finite automata components involving the `Tag` class (`src/log_surgeon/finite_automata/Tag.hpp`), it's important to ensure that `Tag*` pointers in other objects cannot be `nullptr`. Test cases should focus on validating that these `Tag*` pointers are not null where they are used, and handle `nullptr` appropriately.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-26T09:51:07.213Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:123-159
Timestamp: 2025-08-26T09:51:07.213Z
Learning: In log-surgeon's wildcard_query_parser, the ByteLexer's get_matching_variable_types method returns variable type IDs already ordered by priority (highest to lowest), so selecting the first ID gives the highest priority match.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
🧬 Code graph analysis (1)
src/log_surgeon/wildcard_query_parser/Query.cpp (2)
src/log_surgeon/wildcard_query_parser/Query.hpp (4)
Query(16-16)lexer(72-73)expression_view(109-112)regex_string(125-126)src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp (1)
suffix(79-79)
⏰ 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). (6)
- GitHub Check: build (macos-15, debug)
- GitHub Check: build (ubuntu-22.04, debug)
- GitHub Check: build (ubuntu-24.04, release)
- GitHub Check: build (macos-15, release)
- GitHub Check: lint (macos-15)
- GitHub Check: lint (ubuntu-24.04)
🔇 Additional comments (2)
src/log_surgeon/wildcard_query_parser/Query.cpp (2)
36-76: Constructor logic looks solid (escape handling + wildcard collapsing).Pre-reserving capacity and collapsing mixed wildcard runs to a single '*' are correct and efficient.
148-157: Priority-respecting single-type selection is correct.Breaking after the first match when no wildcards are present aligns with the lexer’s priority-ordered IDs (per project context).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/log_surgeon/wildcard_query_parser/Query.hpp (1)
41-44: Use consistent [a,b) interval notation and fix stray double space.Aligns the example with the [a,b) convention and removes an extra space.
Apply:
- * - Example: in "a*b", substring (0,1] extends to "a*", therefore substring [0,2) "a*" is - * redundant. In other words, a decomposition like "a*" + "b" is a subset of the more + * - Example: in "a*b", substring [0,1) extends to "a*", therefore substring [0,2) "a*" is + * redundant. In other words, a decomposition like "a*" + "b" is a subset of the more
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/log_surgeon/wildcard_query_parser/Query.hpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,h,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/log_surgeon/wildcard_query_parser/Query.hpp
🧠 Learnings (17)
📓 Common learnings
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
📚 Learning: 2025-08-08T10:21:56.571Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:57-77
Timestamp: 2025-08-08T10:21:56.571Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp/.cpp (C++), for QueryInterpretation::append_query_interpretation, prefer a single overload taking QueryInterpretation const&; do not suggest adding an rvalue/move overload in future reviews for this method.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:00:20.963Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:00:20.963Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp (C++), do not default the comparison operators: the class stores std::vector<std::variant<StaticQueryToken, VariableQueryToken>>, which yields only a weak ordering. A custom operator<=> that maps the variant’s weak ordering to std::strong_ordering is required; avoid suggesting defaulting there in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:23:06.281Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:80-88
Timestamp: 2025-08-08T10:23:06.281Z
Learning: In y-scope/log-surgeon (C++), small function definitions are allowed to remain inline in headers. For src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp, do not suggest moving small methods like QueryInterpretation::append_variable_token to the .cpp for “consistency” in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:17:43.495Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:17:43.495Z
Learning: In src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp/.cpp (C++), do not suggest defaulting operator<=>. The project prefers a custom out-of-line comparator that explicitly handles the bool member (via explicit cast) to avoid implicit conversions; keep the current manual implementation.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:22:26.739Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:98-120
Timestamp: 2025-08-08T10:22:26.739Z
Learning: In C++ file src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp, for QueryInterpretation::serialize(), prefer explicit std::holds_alternative/std::get branching over std::visit for readability; do not suggest refactoring to std::visit in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-13T12:03:18.145Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:32-37
Timestamp: 2025-08-13T12:03:18.145Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the extend_to_adjacent_greedy_wildcards() method should only extend by one greedy wildcard character on each side, not multiple consecutive ones. This single-wildcard extension is the intended behavior for search operations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-15T00:13:46.717Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: tests/test-wildcard-expression.cpp:0-0
Timestamp: 2025-08-15T00:13:46.717Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpression.cpp, the escaping logic treats consecutive backslashes as escape-literal pairs, not as individual escape characters. For input like R"(a\*b\?c\\)", position 7 is the escape character and position 8 is the literal backslash being escaped, so only position 7 should have is_escape() return true.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-13T12:05:00.245Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:05:00.245Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the generate_regex_string() method should not enforce well-formedness checks via assertions. The is_well_formed() method is intended to be used by callers at their discretion, allowing flexibility to generate regex strings from malformed views if desired.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T09:31:46.313Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T09:31:28.567Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:28.567Z
Learning: In src/log_surgeon/wildcard_query_parser, escaped wildcards (\* or \?) lose their wildcard nature entirely and become literal characters, while escaped delimiters retain their delimiter property in the delimiter table regardless of being escaped. This means boundary detection logic correctly treats escaped wildcards as non-wildcards but escaped delimiters as still being delimiters.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T13:18:39.895Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:42-48
Timestamp: 2025-08-08T13:18:39.895Z
Learning: In y-scope/log-surgeon (C++), it is acceptable/preferred to keep `const` qualifiers on by-value function parameters to signal intent (e.g., in src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp). Do not suggest removing `const` from by-value parameters in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-13T12:06:36.584Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:06:36.584Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, caching SchemaParser::get_special_regex_characters() with a const& reference provides no performance benefit, indicating the method likely returns a reference to an existing container rather than performing expensive computations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T09:51:07.213Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:123-159
Timestamp: 2025-08-26T09:51:07.213Z
Learning: In log-surgeon's wildcard_query_parser, the ByteLexer's get_matching_variable_types method returns variable type IDs already ordered by priority (highest to lowest), so selecting the first ID gives the highest priority match.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-25T20:44:48.955Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:159-177
Timestamp: 2025-08-25T20:44:48.955Z
Learning: In the log-surgeon codebase, thread safety issues with global state like NonTerminal::m_next_children_start should be addressed comprehensively in dedicated PRs rather than fixed piecemeal in individual feature PRs. The user SharafMohamed prefers to defer such systemic architectural issues to separate PRs.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T10:06:22.914Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.hpp:9-11
Timestamp: 2025-08-26T10:06:22.914Z
Learning: In y-scope/log-surgeon project, it's acceptable to include headers like log_surgeon/Lexer.hpp directly in header files rather than using forward declarations, even when only references are used in the interface. The project prefers the simplicity of direct includes over header coupling optimization through forward declarations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-15T01:04:03.681Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/ExpressionView.cpp:11-11
Timestamp: 2025-08-15T01:04:03.681Z
Learning: In y-scope/log-surgeon C++ codebase, prefer explicit includes in implementation files (.cpp) for types that are directly used, even when those headers are already transitively included through the corresponding header file. This makes dependencies explicit and improves maintainability.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
🧬 Code graph analysis (1)
src/log_surgeon/wildcard_query_parser/Query.hpp (4)
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp (3)
nodiscard(41-44)nodiscard(105-105)nodiscard(108-113)src/log_surgeon/wildcard_query_parser/ExpressionCharacter.hpp (7)
nodiscard(21-21)nodiscard(23-23)nodiscard(25-27)nodiscard(29-31)nodiscard(33-35)nodiscard(37-40)nodiscard(42-42)src/log_surgeon/wildcard_query_parser/Expression.hpp (3)
nodiscard(22-24)nodiscard(26-26)nodiscard(28-28)src/log_surgeon/Lexer.hpp (3)
nodiscard(153-153)nodiscard(155-157)nodiscard(159-161)
🔇 Additional comments (1)
src/log_surgeon/wildcard_query_parser/Query.hpp (1)
75-77: Good: avoid copies and name reflects intent.Returning a const reference and renaming the member to m_processed_query_string is clear and efficient.
Also applies to: 128-128
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/log_surgeon/wildcard_query_parser/Query.hpp (1)
18-21: Clarify “n-length” → “n-character” (regression).- * Generates all multi-token interpretations of the n-length query string (single-token + * Generates all multi-token interpretations of the n-character query string (single-token
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/log_surgeon/wildcard_query_parser/Query.hpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,h,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/log_surgeon/wildcard_query_parser/Query.hpp
🧠 Learnings (17)
📓 Common learnings
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:28.567Z
Learning: In src/log_surgeon/wildcard_query_parser, escaped wildcards (\* or \?) lose their wildcard nature entirely and become literal characters, while escaped delimiters retain their delimiter property in the delimiter table regardless of being escaped. This means boundary detection logic correctly treats escaped wildcards as non-wildcards but escaped delimiters as still being delimiters.
📚 Learning: 2025-08-08T10:21:56.571Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:57-77
Timestamp: 2025-08-08T10:21:56.571Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp/.cpp (C++), for QueryInterpretation::append_query_interpretation, prefer a single overload taking QueryInterpretation const&; do not suggest adding an rvalue/move overload in future reviews for this method.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:00:20.963Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:00:20.963Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp (C++), do not default the comparison operators: the class stores std::vector<std::variant<StaticQueryToken, VariableQueryToken>>, which yields only a weak ordering. A custom operator<=> that maps the variant’s weak ordering to std::strong_ordering is required; avoid suggesting defaulting there in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:23:06.281Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:80-88
Timestamp: 2025-08-08T10:23:06.281Z
Learning: In y-scope/log-surgeon (C++), small function definitions are allowed to remain inline in headers. For src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp, do not suggest moving small methods like QueryInterpretation::append_variable_token to the .cpp for “consistency” in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:17:43.495Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:17:43.495Z
Learning: In src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp/.cpp (C++), do not suggest defaulting operator<=>. The project prefers a custom out-of-line comparator that explicitly handles the bool member (via explicit cast) to avoid implicit conversions; keep the current manual implementation.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:22:26.739Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:98-120
Timestamp: 2025-08-08T10:22:26.739Z
Learning: In C++ file src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp, for QueryInterpretation::serialize(), prefer explicit std::holds_alternative/std::get branching over std::visit for readability; do not suggest refactoring to std::visit in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-13T12:03:18.145Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:32-37
Timestamp: 2025-08-13T12:03:18.145Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the extend_to_adjacent_greedy_wildcards() method should only extend by one greedy wildcard character on each side, not multiple consecutive ones. This single-wildcard extension is the intended behavior for search operations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-15T00:13:46.717Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: tests/test-wildcard-expression.cpp:0-0
Timestamp: 2025-08-15T00:13:46.717Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpression.cpp, the escaping logic treats consecutive backslashes as escape-literal pairs, not as individual escape characters. For input like R"(a\*b\?c\\)", position 7 is the escape character and position 8 is the literal backslash being escaped, so only position 7 should have is_escape() return true.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-13T12:05:00.245Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:05:00.245Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the generate_regex_string() method should not enforce well-formedness checks via assertions. The is_well_formed() method is intended to be used by callers at their discretion, allowing flexibility to generate regex strings from malformed views if desired.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T09:31:46.313Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T09:31:28.567Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:28.567Z
Learning: In src/log_surgeon/wildcard_query_parser, escaped wildcards (\* or \?) lose their wildcard nature entirely and become literal characters, while escaped delimiters retain their delimiter property in the delimiter table regardless of being escaped. This means boundary detection logic correctly treats escaped wildcards as non-wildcards but escaped delimiters as still being delimiters.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T13:18:39.895Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:42-48
Timestamp: 2025-08-08T13:18:39.895Z
Learning: In y-scope/log-surgeon (C++), it is acceptable/preferred to keep `const` qualifiers on by-value function parameters to signal intent (e.g., in src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp). Do not suggest removing `const` from by-value parameters in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-13T12:06:36.584Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:06:36.584Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, caching SchemaParser::get_special_regex_characters() with a const& reference provides no performance benefit, indicating the method likely returns a reference to an existing container rather than performing expensive computations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T09:51:07.213Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:123-159
Timestamp: 2025-08-26T09:51:07.213Z
Learning: In log-surgeon's wildcard_query_parser, the ByteLexer's get_matching_variable_types method returns variable type IDs already ordered by priority (highest to lowest), so selecting the first ID gives the highest priority match.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-25T20:44:48.955Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:159-177
Timestamp: 2025-08-25T20:44:48.955Z
Learning: In the log-surgeon codebase, thread safety issues with global state like NonTerminal::m_next_children_start should be addressed comprehensively in dedicated PRs rather than fixed piecemeal in individual feature PRs. The user SharafMohamed prefers to defer such systemic architectural issues to separate PRs.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T10:06:22.914Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.hpp:9-11
Timestamp: 2025-08-26T10:06:22.914Z
Learning: In y-scope/log-surgeon project, it's acceptable to include headers like log_surgeon/Lexer.hpp directly in header files rather than using forward declarations, even when only references are used in the interface. The project prefers the simplicity of direct includes over header coupling optimization through forward declarations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-15T01:04:03.681Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/ExpressionView.cpp:11-11
Timestamp: 2025-08-15T01:04:03.681Z
Learning: In y-scope/log-surgeon C++ codebase, prefer explicit includes in implementation files (.cpp) for types that are directly used, even when those headers are already transitively included through the corresponding header file. This makes dependencies explicit and improves maintainability.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
🧬 Code graph analysis (1)
src/log_surgeon/wildcard_query_parser/Query.hpp (3)
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp (3)
nodiscard(41-44)nodiscard(105-105)nodiscard(108-113)src/log_surgeon/wildcard_query_parser/ExpressionCharacter.hpp (7)
nodiscard(21-21)nodiscard(23-23)nodiscard(25-27)nodiscard(29-31)nodiscard(33-35)nodiscard(37-40)nodiscard(42-42)src/log_surgeon/Lexer.hpp (6)
nodiscard(153-153)nodiscard(155-157)nodiscard(159-161)nodiscard(163-166)nodiscard(175-181)nodiscard(188-194)
🔇 Additional comments (1)
src/log_surgeon/wildcard_query_parser/Query.hpp (1)
120-133: Remove vector return type verification – signature unchanged
The methodget_matching_variable_typesstill returnsstd::set<uint32_t>, so the suggested checks for astd::vector<uint32_t>signature and its call sites are unnecessary.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp (1)
105-113: Expose view indices to callers (needed for correct DP accounting)Query.cpp needs the extended view’s [begin,end) to place results in the right DP buckets. With get_indices() private, callers cannot adjust. Provide a small public wrapper.
Apply:
[[nodiscard]] auto get_search_string() const -> std::string_view { return m_search_string; } + // Public wrapper to expose [begin,end) for DP algorithms without leaking internals. + [[nodiscard]] auto indices() const -> std::pair<size_t, size_t> { return get_indices(); } + private: [[nodiscard]] auto get_indices() const -> std::pair<size_t, size_t> { auto const& full_chars{m_expression->get_chars()}; auto const* begin_ptr{m_chars.data()}; auto begin_idx{static_cast<size_t>(begin_ptr - full_chars.data())}; return {begin_idx, begin_idx + m_chars.size()}; }Also applies to: 115-118
♻️ Duplicate comments (3)
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp (1)
46-68: Align boundary docs with escape semantics and empty-view caveatDocs still say “delimiter or wildcard” and omit the “(if non-empty)” qualifiers. To match the established semantics (escaped wildcards are literals; escaped delimiters remain delimiters; empty views don’t have first/last chars), please tighten the wording.
Apply:
@@ - * Left boundary: - * - The view is at the start of the expression, or - * - The first character is a greedy wildcard, or - * - Immediately left of the view is a delimiter or wildcard. + * Left boundary: + * - The view is at the start of the expression, or + * - The first character is a greedy wildcard (if non-empty), or + * - Immediately left of the view is a delimiter or an unescaped wildcard. @@ - * Right boundary: - * - The view is at the end of the expression, or - * - The last character is a greedy wildcard, or - * - Immediately right of the view is a delimiter or wildcard, or - * - Immediately right of the view is an escape character and the character to its - * immediate right is a delimiter. + * Right boundary: + * - The view is at the end of the expression, or + * - The last character is a greedy wildcard (if non-empty), or + * - Immediately right of the view is a delimiter or an unescaped wildcard, or + * - Immediately right of the view is an escape character and the character to its + * immediate right is a delimiter.src/log_surgeon/wildcard_query_parser/Query.cpp (2)
160-175: Global state reset remains non-reentrant (tracked separately)NonTerminal::m_next_children_start is reset without restore, which is unsafe under concurrency. I understand this is tracked in issue #154 and will be fixed holistically.
No action for this PR; just ensuring it stays linked to the meta thread-safety fix.
135-138: Confirm boundary helper matches intent (wildcards count as boundaries)This check relies on ExpressionView treating wildcards as boundaries. If not, switch to the helper that includes wildcards, or update the helper’s docs/name per the header comment.
If needed:
- if (false == expression_view.is_surrounded_by_delims(lexer.get_delim_table())) { + if (false == expression_view.is_surrounded_by_delims_or_wildcards(lexer.get_delim_table())) {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp(2 hunks)src/log_surgeon/wildcard_query_parser/Query.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,h,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
🧠 Learnings (18)
📓 Common learnings
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:28.567Z
Learning: In src/log_surgeon/wildcard_query_parser, escaped wildcards (\* or \?) lose their wildcard nature entirely and become literal characters, while escaped delimiters retain their delimiter property in the delimiter table regardless of being escaped. This means boundary detection logic correctly treats escaped wildcards as non-wildcards but escaped delimiters as still being delimiters.
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
📚 Learning: 2025-08-08T10:21:56.571Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:57-77
Timestamp: 2025-08-08T10:21:56.571Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp/.cpp (C++), for QueryInterpretation::append_query_interpretation, prefer a single overload taking QueryInterpretation const&; do not suggest adding an rvalue/move overload in future reviews for this method.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-08T10:00:20.963Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:00:20.963Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp (C++), do not default the comparison operators: the class stores std::vector<std::variant<StaticQueryToken, VariableQueryToken>>, which yields only a weak ordering. A custom operator<=> that maps the variant’s weak ordering to std::strong_ordering is required; avoid suggesting defaulting there in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-08T10:23:06.281Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:80-88
Timestamp: 2025-08-08T10:23:06.281Z
Learning: In y-scope/log-surgeon (C++), small function definitions are allowed to remain inline in headers. For src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp, do not suggest moving small methods like QueryInterpretation::append_variable_token to the .cpp for “consistency” in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-08T10:17:43.495Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:17:43.495Z
Learning: In src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp/.cpp (C++), do not suggest defaulting operator<=>. The project prefers a custom out-of-line comparator that explicitly handles the bool member (via explicit cast) to avoid implicit conversions; keep the current manual implementation.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-08T10:22:26.739Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:98-120
Timestamp: 2025-08-08T10:22:26.739Z
Learning: In C++ file src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp, for QueryInterpretation::serialize(), prefer explicit std::holds_alternative/std::get branching over std::visit for readability; do not suggest refactoring to std::visit in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-13T12:03:18.145Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:32-37
Timestamp: 2025-08-13T12:03:18.145Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the extend_to_adjacent_greedy_wildcards() method should only extend by one greedy wildcard character on each side, not multiple consecutive ones. This single-wildcard extension is the intended behavior for search operations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-26T09:31:46.313Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-13T12:05:00.245Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:05:00.245Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the generate_regex_string() method should not enforce well-formedness checks via assertions. The is_well_formed() method is intended to be used by callers at their discretion, allowing flexibility to generate regex strings from malformed views if desired.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-13T12:06:36.584Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:06:36.584Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, caching SchemaParser::get_special_regex_characters() with a const& reference provides no performance benefit, indicating the method likely returns a reference to an existing container rather than performing expensive computations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-15T00:13:46.717Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: tests/test-wildcard-expression.cpp:0-0
Timestamp: 2025-08-15T00:13:46.717Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpression.cpp, the escaping logic treats consecutive backslashes as escape-literal pairs, not as individual escape characters. For input like R"(a\*b\?c\\)", position 7 is the escape character and position 8 is the literal backslash being escaped, so only position 7 should have is_escape() return true.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-08T13:18:39.895Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:42-48
Timestamp: 2025-08-08T13:18:39.895Z
Learning: In y-scope/log-surgeon (C++), it is acceptable/preferred to keep `const` qualifiers on by-value function parameters to signal intent (e.g., in src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp). Do not suggest removing `const` from by-value parameters in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-25T20:44:48.955Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:159-177
Timestamp: 2025-08-25T20:44:48.955Z
Learning: In the log-surgeon codebase, thread safety issues with global state like NonTerminal::m_next_children_start should be addressed comprehensively in dedicated PRs rather than fixed piecemeal in individual feature PRs. The user SharafMohamed prefers to defer such systemic architectural issues to separate PRs.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2024-10-24T15:54:35.193Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#42
File: src/log_surgeon/finite_automata/RegexNFA.hpp:442-456
Timestamp: 2024-10-24T15:54:35.193Z
Learning: In the C++ file `src/log_surgeon/finite_automata/RegexNFA.hpp`, for the `RegexNFA::serialize()` function, prioritize code clarity over efficiency when handling string operations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-26T09:31:28.567Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:28.567Z
Learning: In src/log_surgeon/wildcard_query_parser, escaped wildcards (\* or \?) lose their wildcard nature entirely and become literal characters, while escaped delimiters retain their delimiter property in the delimiter table regardless of being escaped. This means boundary detection logic correctly treats escaped wildcards as non-wildcards but escaped delimiters as still being delimiters.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2024-11-13T22:25:54.168Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#48
File: tests/test-tag.cpp:10-10
Timestamp: 2024-11-13T22:25:54.168Z
Learning: In the log-surgeon codebase (C++), particularly in the finite automata components involving the `Tag` class (`src/log_surgeon/finite_automata/Tag.hpp`), it's important to ensure that `Tag*` pointers in other objects cannot be `nullptr`. Test cases should focus on validating that these `Tag*` pointers are not null where they are used, and handle `nullptr` appropriately.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-26T09:51:07.213Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:123-159
Timestamp: 2025-08-26T09:51:07.213Z
Learning: In log-surgeon's wildcard_query_parser, the ByteLexer's get_matching_variable_types method returns variable type IDs already ordered by priority (highest to lowest), so selecting the first ID gives the highest priority match.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cpp
📚 Learning: 2025-08-13T12:09:51.983Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpression.hpp:0-0
Timestamp: 2025-08-13T12:09:51.983Z
Learning: In y-scope/log-surgeon WildcardExpression/WildcardExpressionView architecture, wildcard-related metadata (like contains_wildcard flags) should be placed in WildcardExpressionView rather than WildcardExpression, since the view is responsible for regex generation and may represent subranges with different wildcard characteristics than the full expression.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.cppsrc/log_surgeon/wildcard_query_parser/ExpressionView.hpp
🧬 Code graph analysis (2)
src/log_surgeon/wildcard_query_parser/Query.cpp (2)
src/log_surgeon/wildcard_query_parser/Query.hpp (4)
Query(16-16)lexer(78-79)expression_view(115-118)regex_string(131-132)src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp (1)
suffix(79-79)
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp (3)
src/log_surgeon/wildcard_query_parser/ExpressionCharacter.hpp (9)
nodiscard(21-21)nodiscard(23-23)nodiscard(25-27)nodiscard(29-31)nodiscard(33-35)nodiscard(37-40)nodiscard(42-42)delim_table(33-33)delim_table(37-38)src/log_surgeon/wildcard_query_parser/Expression.hpp (3)
nodiscard(22-24)nodiscard(26-26)nodiscard(28-28)src/log_surgeon/Lexer.hpp (5)
nodiscard(153-153)nodiscard(155-157)nodiscard(159-161)nodiscard(163-166)nodiscard(175-181)
…ngth refers to characters.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/log_surgeon/wildcard_query_parser/Query.hpp (3)
23-25: Say “adjacent greedy wildcards” for consistency.- * - Substrings adjacent to greedy wildcards must be interpreted as if they include them. To - * implement this, we extend all substrings to include adjacent wildcards. + * - Substrings adjacent to greedy wildcards must be interpreted as if they include them. To + * implement this, we extend all substrings to include adjacent greedy wildcards.
51-53: Wording/grammar: “multi-token” and drop “to” after Let.- * 2. Let I(a) be the set of all k-length interpretations of substring [0,a), where 1 <= k < a. - * - Let T(a,b) to be the set of all valid single-token interpretations of substring [a,b). + * 2. Let I(a) be the set of all multi-token interpretations of substring [0,a), where 1 <= k <= a. + * - Let T(a,b) be the set of all valid single-token interpretations of substring [a,b).
120-133: Preserve lexer priority: return an ordered container, not std::set
Change the signature and implementation so you forward the vector from ByteDfa::get_intersect (which is already ordered by priority) instead of converting it into a sorted std::set:--- a/src/log_surgeon/wildcard_query_parser/Query.hpp @@ - /** - * @return The set of all matching variable type IDs. + /** + * @return Variable type IDs ordered by lexer priority (highest first). @@ - [[nodiscard]] static auto - get_matching_variable_types(std::string const& regex_string, lexers::ByteLexer const& lexer) - -> std::set<uint32_t>; + [[nodiscard]] static auto + get_matching_variable_types(std::string const& regex_string, lexers::ByteLexer const& lexer) + -> std::vector<uint32_t>;And in Query.cpp, update the return type and drop the implicit set–conversion:
--- a/src/log_surgeon/wildcard_query_parser/Query.cpp @@ -auto Query::get_matching_variable_types(string const& regex_string, ByteLexer const& lexer) - -> set<uint32_t> { +auto Query::get_matching_variable_types(string const& regex_string, ByteLexer const& lexer) + -> vector<uint32_t> { NonTerminal::m_next_children_start = 0; … - auto const var_types = lexer.get_dfa()->get_intersect(&dfa); - return var_types; + auto var_types = lexer.get_dfa()->get_intersect(&dfa); + // if duplicates can occur, use a stable dedup to preserve order: + // var_types.erase(std::unique(var_types.begin(), var_types.end()), var_types.end()); + return var_types; }This ensures the first element you pick truly reflects the highest-priority match.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/log_surgeon/wildcard_query_parser/Query.hpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,h,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/log_surgeon/wildcard_query_parser/Query.hpp
🧠 Learnings (16)
📚 Learning: 2025-08-08T10:00:20.963Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:00:20.963Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp (C++), do not default the comparison operators: the class stores std::vector<std::variant<StaticQueryToken, VariableQueryToken>>, which yields only a weak ordering. A custom operator<=> that maps the variant’s weak ordering to std::strong_ordering is required; avoid suggesting defaulting there in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:21:56.571Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:57-77
Timestamp: 2025-08-08T10:21:56.571Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp/.cpp (C++), for QueryInterpretation::append_query_interpretation, prefer a single overload taking QueryInterpretation const&; do not suggest adding an rvalue/move overload in future reviews for this method.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:23:06.281Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:80-88
Timestamp: 2025-08-08T10:23:06.281Z
Learning: In y-scope/log-surgeon (C++), small function definitions are allowed to remain inline in headers. For src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp, do not suggest moving small methods like QueryInterpretation::append_variable_token to the .cpp for “consistency” in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:17:43.495Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:17:43.495Z
Learning: In src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp/.cpp (C++), do not suggest defaulting operator<=>. The project prefers a custom out-of-line comparator that explicitly handles the bool member (via explicit cast) to avoid implicit conversions; keep the current manual implementation.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:22:26.739Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:98-120
Timestamp: 2025-08-08T10:22:26.739Z
Learning: In C++ file src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp, for QueryInterpretation::serialize(), prefer explicit std::holds_alternative/std::get branching over std::visit for readability; do not suggest refactoring to std::visit in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-13T12:03:18.145Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:32-37
Timestamp: 2025-08-13T12:03:18.145Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the extend_to_adjacent_greedy_wildcards() method should only extend by one greedy wildcard character on each side, not multiple consecutive ones. This single-wildcard extension is the intended behavior for search operations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-15T00:13:46.717Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: tests/test-wildcard-expression.cpp:0-0
Timestamp: 2025-08-15T00:13:46.717Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpression.cpp, the escaping logic treats consecutive backslashes as escape-literal pairs, not as individual escape characters. For input like R"(a\*b\?c\\)", position 7 is the escape character and position 8 is the literal backslash being escaped, so only position 7 should have is_escape() return true.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-13T12:05:00.245Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:05:00.245Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the generate_regex_string() method should not enforce well-formedness checks via assertions. The is_well_formed() method is intended to be used by callers at their discretion, allowing flexibility to generate regex strings from malformed views if desired.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T09:31:46.313Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T09:31:28.567Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:28.567Z
Learning: In src/log_surgeon/wildcard_query_parser, escaped wildcards (\* or \?) lose their wildcard nature entirely and become literal characters, while escaped delimiters retain their delimiter property in the delimiter table regardless of being escaped. This means boundary detection logic correctly treats escaped wildcards as non-wildcards but escaped delimiters as still being delimiters.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T13:18:39.895Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:42-48
Timestamp: 2025-08-08T13:18:39.895Z
Learning: In y-scope/log-surgeon (C++), it is acceptable/preferred to keep `const` qualifiers on by-value function parameters to signal intent (e.g., in src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp). Do not suggest removing `const` from by-value parameters in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-13T12:06:36.584Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:06:36.584Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, caching SchemaParser::get_special_regex_characters() with a const& reference provides no performance benefit, indicating the method likely returns a reference to an existing container rather than performing expensive computations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T09:51:07.213Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:123-159
Timestamp: 2025-08-26T09:51:07.213Z
Learning: In log-surgeon's wildcard_query_parser, the ByteLexer's get_matching_variable_types method returns variable type IDs already ordered by priority (highest to lowest), so selecting the first ID gives the highest priority match.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-25T20:44:48.955Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:159-177
Timestamp: 2025-08-25T20:44:48.955Z
Learning: In the log-surgeon codebase, thread safety issues with global state like NonTerminal::m_next_children_start should be addressed comprehensively in dedicated PRs rather than fixed piecemeal in individual feature PRs. The user SharafMohamed prefers to defer such systemic architectural issues to separate PRs.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T10:06:22.914Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.hpp:9-11
Timestamp: 2025-08-26T10:06:22.914Z
Learning: In y-scope/log-surgeon project, it's acceptable to include headers like log_surgeon/Lexer.hpp directly in header files rather than using forward declarations, even when only references are used in the interface. The project prefers the simplicity of direct includes over header coupling optimization through forward declarations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-15T01:04:03.681Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/ExpressionView.cpp:11-11
Timestamp: 2025-08-15T01:04:03.681Z
Learning: In y-scope/log-surgeon C++ codebase, prefer explicit includes in implementation files (.cpp) for types that are directly used, even when those headers are already transitively included through the corresponding header file. This makes dependencies explicit and improves maintainability.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
🧬 Code graph analysis (1)
src/log_surgeon/wildcard_query_parser/Query.hpp (3)
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp (3)
nodiscard(41-44)nodiscard(105-105)nodiscard(108-113)src/log_surgeon/wildcard_query_parser/ExpressionCharacter.hpp (7)
nodiscard(21-21)nodiscard(23-23)nodiscard(25-27)nodiscard(29-31)nodiscard(33-35)nodiscard(37-40)nodiscard(42-42)src/log_surgeon/Lexer.hpp (6)
nodiscard(153-153)nodiscard(155-157)nodiscard(159-161)nodiscard(163-166)nodiscard(175-181)nodiscard(188-194)
⏰ 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). (7)
- GitHub Check: lint (ubuntu-24.04)
- GitHub Check: lint (macos-15)
- GitHub Check: build (ubuntu-24.04, debug)
- GitHub Check: build (macos-15, debug)
- GitHub Check: build (ubuntu-24.04, release)
- GitHub Check: build (macos-15, release)
- GitHub Check: build (ubuntu-22.04, debug)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp (1)
69-71: Name can mislead: method also treats wildcards as boundariesMethod name mentions only delimiters while the contract includes wildcards. If feasible, rename; otherwise, explicitly reinforce in the first line that wildcards are also considered boundaries.
Option A (rename):
- [[nodiscard]] auto is_surrounded_by_delims( + [[nodiscard]] auto is_surrounded_by_delims_or_wildcards( std::array<bool, cSizeOfByte> const& delim_table ) const -> bool;Option B (doc note right above the declaration):
- /** - * Checks whether the view is surrounded by delimiters. The start and end of an expression are + /** + * Checks whether the view is surrounded by delimiters (wildcards also qualify as boundaries). + * The start and end of an expression are
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,h,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp
🧠 Learnings (12)
📓 Common learnings
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:57-77
Timestamp: 2025-08-08T10:21:56.571Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp/.cpp (C++), for QueryInterpretation::append_query_interpretation, prefer a single overload taking QueryInterpretation const&; do not suggest adding an rvalue/move overload in future reviews for this method.
📚 Learning: 2025-08-26T09:31:46.313Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-13T12:05:00.245Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:05:00.245Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the generate_regex_string() method should not enforce well-formedness checks via assertions. The is_well_formed() method is intended to be used by callers at their discretion, allowing flexibility to generate regex strings from malformed views if desired.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-13T12:03:18.145Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:32-37
Timestamp: 2025-08-13T12:03:18.145Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the extend_to_adjacent_greedy_wildcards() method should only extend by one greedy wildcard character on each side, not multiple consecutive ones. This single-wildcard extension is the intended behavior for search operations.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-13T12:09:51.983Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpression.hpp:0-0
Timestamp: 2025-08-13T12:09:51.983Z
Learning: In y-scope/log-surgeon WildcardExpression/WildcardExpressionView architecture, wildcard-related metadata (like contains_wildcard flags) should be placed in WildcardExpressionView rather than WildcardExpression, since the view is responsible for regex generation and may represent subranges with different wildcard characteristics than the full expression.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-26T09:31:28.567Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:28.567Z
Learning: In src/log_surgeon/wildcard_query_parser, escaped wildcards (\* or \?) lose their wildcard nature entirely and become literal characters, while escaped delimiters retain their delimiter property in the delimiter table regardless of being escaped. This means boundary detection logic correctly treats escaped wildcards as non-wildcards but escaped delimiters as still being delimiters.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-15T00:13:46.717Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: tests/test-wildcard-expression.cpp:0-0
Timestamp: 2025-08-15T00:13:46.717Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpression.cpp, the escaping logic treats consecutive backslashes as escape-literal pairs, not as individual escape characters. For input like R"(a\*b\?c\\)", position 7 is the escape character and position 8 is the literal backslash being escaped, so only position 7 should have is_escape() return true.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-08T10:21:56.571Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:57-77
Timestamp: 2025-08-08T10:21:56.571Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp/.cpp (C++), for QueryInterpretation::append_query_interpretation, prefer a single overload taking QueryInterpretation const&; do not suggest adding an rvalue/move overload in future reviews for this method.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-08T10:00:20.963Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:00:20.963Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp (C++), do not default the comparison operators: the class stores std::vector<std::variant<StaticQueryToken, VariableQueryToken>>, which yields only a weak ordering. A custom operator<=> that maps the variant’s weak ordering to std::strong_ordering is required; avoid suggesting defaulting there in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-09-03T14:27:18.143Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:0-0
Timestamp: 2025-09-03T14:27:18.143Z
Learning: In the log_surgeon wildcard_query_parser module, the correct terminology is: a "wildcard" refers specifically to unescaped * or ? characters with special meaning, while escaped \* or \? characters are just "literal characters" or "literals", not "escaped wildcards". Using "unescaped wildcard" is redundant since wildcards are by definition unescaped.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-08T13:18:39.895Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:42-48
Timestamp: 2025-08-08T13:18:39.895Z
Learning: In y-scope/log-surgeon (C++), it is acceptable/preferred to keep `const` qualifiers on by-value function parameters to signal intent (e.g., in src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp). Do not suggest removing `const` from by-value parameters in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp
📚 Learning: 2025-08-13T12:06:36.584Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:06:36.584Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, caching SchemaParser::get_special_regex_characters() with a const& reference provides no performance benefit, indicating the method likely returns a reference to an existing container rather than performing expensive computations.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp
🧬 Code graph analysis (1)
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp (3)
src/log_surgeon/wildcard_query_parser/ExpressionCharacter.hpp (9)
nodiscard(21-21)nodiscard(23-23)nodiscard(25-27)nodiscard(29-31)nodiscard(33-35)nodiscard(37-40)nodiscard(42-42)delim_table(33-33)delim_table(37-38)src/log_surgeon/Lexer.hpp (5)
nodiscard(153-153)nodiscard(155-157)nodiscard(159-161)nodiscard(163-166)nodiscard(175-181)src/log_surgeon/wildcard_query_parser/Expression.hpp (3)
nodiscard(22-24)nodiscard(26-26)nodiscard(28-28)
⏰ 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). (6)
- GitHub Check: build (ubuntu-24.04, release)
- GitHub Check: build (macos-15, debug)
- GitHub Check: build (ubuntu-22.04, debug)
- GitHub Check: build (ubuntu-24.04, debug)
- GitHub Check: build (macos-15, release)
- GitHub Check: lint (macos-15)
🔇 Additional comments (1)
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp (1)
4-4: Includes look correct and minimalAdding and Constants.hpp is appropriate for the delim table and cSizeOfByte usage in this header.
Also applies to: 11-11
… as thats impossible.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
tests/test-query.cpp (2)
377-381: Repeat of the same typo (“aggresively”)Same correction as above.
- * NOTE: Similar to the above `int_query` test there are unneeded interpretations due to aggresively + * NOTE: Similar to the above `int_query` test there are unneeded interpretations due to aggressively
331-333: Typos in note (“aggresively”)Correct spelling.
- * NOTE: Similar to the above `int_query` test there are unneeded interpretations due to aggresively + * NOTE: Similar to the above `int_query` test there are unneeded interpretations due to aggressively
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/log_surgeon/wildcard_query_parser/ExpressionView.cpp(2 hunks)tests/test-query.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,h,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cpptests/test-query.cpp
🧠 Learnings (22)
📓 Common learnings
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:57-77
Timestamp: 2025-08-08T10:21:56.571Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp/.cpp (C++), for QueryInterpretation::append_query_interpretation, prefer a single overload taking QueryInterpretation const&; do not suggest adding an rvalue/move overload in future reviews for this method.
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:00:20.963Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp (C++), do not default the comparison operators: the class stores std::vector<std::variant<StaticQueryToken, VariableQueryToken>>, which yields only a weak ordering. A custom operator<=> that maps the variant’s weak ordering to std::strong_ordering is required; avoid suggesting defaulting there in future reviews.
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:17:43.495Z
Learning: In src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp/.cpp (C++), do not suggest defaulting operator<=>. The project prefers a custom out-of-line comparator that explicitly handles the bool member (via explicit cast) to avoid implicit conversions; keep the current manual implementation.
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:135-138
Timestamp: 2025-09-03T16:29:13.659Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.cpp, the is_surrounded_by_delims() method actually considers both delimiters and wildcards as valid boundaries, despite the method name suggesting only delimiters. The implementation uses is_delim_or_wildcard() checks for boundary detection.
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:135-138
Timestamp: 2025-09-03T16:29:13.659Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.cpp, the is_surrounded_by_delims() method actually considers both delimiters and wildcards as valid boundaries, despite the method name suggesting only delimiters. The implementation uses is_delim_or_wildcard() checks for boundary detection.
📚 Learning: 2025-09-03T16:29:13.659Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:135-138
Timestamp: 2025-09-03T16:29:13.659Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.cpp, the is_surrounded_by_delims() method actually considers both delimiters and wildcards as valid boundaries, despite the method name suggesting only delimiters. The implementation uses is_delim_or_wildcard() checks for boundary detection.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cpp
📚 Learning: 2025-08-26T09:31:46.313Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cpptests/test-query.cpp
📚 Learning: 2025-08-13T12:03:18.145Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:32-37
Timestamp: 2025-08-13T12:03:18.145Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the extend_to_adjacent_greedy_wildcards() method should only extend by one greedy wildcard character on each side, not multiple consecutive ones. This single-wildcard extension is the intended behavior for search operations.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cpptests/test-query.cpp
📚 Learning: 2025-08-13T12:05:00.245Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:05:00.245Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the generate_regex_string() method should not enforce well-formedness checks via assertions. The is_well_formed() method is intended to be used by callers at their discretion, allowing flexibility to generate regex strings from malformed views if desired.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cpptests/test-query.cpp
📚 Learning: 2025-08-26T09:31:28.567Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:28.567Z
Learning: In src/log_surgeon/wildcard_query_parser, escaped wildcards (\* or \?) lose their wildcard nature entirely and become literal characters, while escaped delimiters retain their delimiter property in the delimiter table regardless of being escaped. This means boundary detection logic correctly treats escaped wildcards as non-wildcards but escaped delimiters as still being delimiters.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cpptests/test-query.cpp
📚 Learning: 2025-08-13T12:09:51.983Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpression.hpp:0-0
Timestamp: 2025-08-13T12:09:51.983Z
Learning: In y-scope/log-surgeon WildcardExpression/WildcardExpressionView architecture, wildcard-related metadata (like contains_wildcard flags) should be placed in WildcardExpressionView rather than WildcardExpression, since the view is responsible for regex generation and may represent subranges with different wildcard characteristics than the full expression.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cpp
📚 Learning: 2025-08-15T00:13:46.717Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: tests/test-wildcard-expression.cpp:0-0
Timestamp: 2025-08-15T00:13:46.717Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpression.cpp, the escaping logic treats consecutive backslashes as escape-literal pairs, not as individual escape characters. For input like R"(a\*b\?c\\)", position 7 is the escape character and position 8 is the literal backslash being escaped, so only position 7 should have is_escape() return true.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cpptests/test-query.cpp
📚 Learning: 2025-08-13T12:06:36.584Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:06:36.584Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, caching SchemaParser::get_special_regex_characters() with a const& reference provides no performance benefit, indicating the method likely returns a reference to an existing container rather than performing expensive computations.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cpptests/test-query.cpp
📚 Learning: 2025-08-08T10:23:06.281Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:80-88
Timestamp: 2025-08-08T10:23:06.281Z
Learning: In y-scope/log-surgeon (C++), small function definitions are allowed to remain inline in headers. For src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp, do not suggest moving small methods like QueryInterpretation::append_variable_token to the .cpp for “consistency” in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cpptests/test-query.cpp
📚 Learning: 2025-08-08T13:18:39.895Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:42-48
Timestamp: 2025-08-08T13:18:39.895Z
Learning: In y-scope/log-surgeon (C++), it is acceptable/preferred to keep `const` qualifiers on by-value function parameters to signal intent (e.g., in src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp). Do not suggest removing `const` from by-value parameters in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cpptests/test-query.cpp
📚 Learning: 2025-08-08T10:22:26.739Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:98-120
Timestamp: 2025-08-08T10:22:26.739Z
Learning: In C++ file src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp, for QueryInterpretation::serialize(), prefer explicit std::holds_alternative/std::get branching over std::visit for readability; do not suggest refactoring to std::visit in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cpptests/test-query.cpp
📚 Learning: 2025-08-08T10:00:20.963Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:00:20.963Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp (C++), do not default the comparison operators: the class stores std::vector<std::variant<StaticQueryToken, VariableQueryToken>>, which yields only a weak ordering. A custom operator<=> that maps the variant’s weak ordering to std::strong_ordering is required; avoid suggesting defaulting there in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cpptests/test-query.cpp
📚 Learning: 2025-08-08T10:21:56.571Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:57-77
Timestamp: 2025-08-08T10:21:56.571Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp/.cpp (C++), for QueryInterpretation::append_query_interpretation, prefer a single overload taking QueryInterpretation const&; do not suggest adding an rvalue/move overload in future reviews for this method.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cpptests/test-query.cpp
📚 Learning: 2025-08-08T10:17:43.495Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:17:43.495Z
Learning: In src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp/.cpp (C++), do not suggest defaulting operator<=>. The project prefers a custom out-of-line comparator that explicitly handles the bool member (via explicit cast) to avoid implicit conversions; keep the current manual implementation.
Applied to files:
src/log_surgeon/wildcard_query_parser/ExpressionView.cpptests/test-query.cpp
📚 Learning: 2025-08-26T10:13:00.368Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: tests/test-query.cpp:61-67
Timestamp: 2025-08-26T10:13:00.368Z
Learning: In Catch2 unit tests, the REQUIRE macro already provides detailed debugging output when container comparisons (like std::set equality) fail, showing both expected and actual values. Additional CAPTURE statements are typically unnecessary for such comparisons.
Applied to files:
tests/test-query.cpp
📚 Learning: 2024-11-13T22:25:54.168Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#48
File: tests/test-tag.cpp:10-10
Timestamp: 2024-11-13T22:25:54.168Z
Learning: In the log-surgeon codebase (C++), particularly in the finite automata components involving the `Tag` class (`src/log_surgeon/finite_automata/Tag.hpp`), it's important to ensure that `Tag*` pointers in other objects cannot be `nullptr`. Test cases should focus on validating that these `Tag*` pointers are not null where they are used, and handle `nullptr` appropriately.
Applied to files:
tests/test-query.cpp
📚 Learning: 2025-08-25T20:44:48.955Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:159-177
Timestamp: 2025-08-25T20:44:48.955Z
Learning: In the log-surgeon codebase, thread safety issues with global state like NonTerminal::m_next_children_start should be addressed comprehensively in dedicated PRs rather than fixed piecemeal in individual feature PRs. The user SharafMohamed prefers to defer such systemic architectural issues to separate PRs.
Applied to files:
tests/test-query.cpp
📚 Learning: 2025-08-15T12:07:58.626Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: tests/test-expression-view.cpp:7-7
Timestamp: 2025-08-15T12:07:58.626Z
Learning: In tests/test-expression-view.cpp, the `<catch2/catch_message.hpp>` header is required for clang-tidy to pass, even though it may not be directly used in the visible code.
Applied to files:
tests/test-query.cpp
📚 Learning: 2025-08-15T01:04:03.681Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/ExpressionView.cpp:11-11
Timestamp: 2025-08-15T01:04:03.681Z
Learning: In y-scope/log-surgeon C++ codebase, prefer explicit includes in implementation files (.cpp) for types that are directly used, even when those headers are already transitively included through the corresponding header file. This makes dependencies explicit and improves maintainability.
Applied to files:
tests/test-query.cpp
📚 Learning: 2025-08-18T12:05:55.905Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#151
File: tests/test-buffer-parser.cpp:0-0
Timestamp: 2025-08-18T12:05:55.905Z
Learning: In y-scope/log-surgeon (C++), prefer uniform initialization with braces {} when possible instead of assignment or parentheses initialization. For example, prefer `string const var{value};` over `string const var = value;`.
Applied to files:
tests/test-query.cpp
📚 Learning: 2024-10-24T15:54:35.193Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#42
File: src/log_surgeon/finite_automata/RegexNFA.hpp:442-456
Timestamp: 2024-10-24T15:54:35.193Z
Learning: In the C++ file `src/log_surgeon/finite_automata/RegexNFA.hpp`, for the `RegexNFA::serialize()` function, prioritize code clarity over efficiency when handling string operations.
Applied to files:
tests/test-query.cpp
🧬 Code graph analysis (2)
src/log_surgeon/wildcard_query_parser/ExpressionView.cpp (2)
src/log_surgeon/wildcard_query_parser/ExpressionView.hpp (1)
delim_table(69-71)src/log_surgeon/wildcard_query_parser/ExpressionCharacter.hpp (2)
delim_table(33-33)delim_table(37-38)
tests/test-query.cpp (3)
src/log_surgeon/SchemaParser.hpp (1)
SchemaVarAST(49-56)src/log_surgeon/wildcard_query_parser/Query.hpp (2)
Query(16-16)lexer(78-79)src/log_surgeon/LogParser.hpp (1)
schema_ast(158-158)
🔇 Additional comments (2)
src/log_surgeon/wildcard_query_parser/ExpressionView.cpp (2)
4-4: Good explicit include for std::arrayRequired for the function signature; keeps dependencies clear.
11-11: Correct dependency for cSizeOfByteIncluding log_surgeon/Constants.hpp here is appropriate given the signature.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/log_surgeon/Lexer.hpp (1)
155-157: Add noexcept to const-ref accessor (repeat).Getter cannot throw; mark it noexcept to match typical accessor semantics and previous review note.
- [[nodiscard]] auto get_delim_table() const -> std::array<bool, cSizeOfByte> const& { + [[nodiscard]] auto get_delim_table() const noexcept -> std::array<bool, cSizeOfByte> const& { return m_is_delimiter; }src/log_surgeon/wildcard_query_parser/Query.hpp (2)
50-52: Fix bounds for I(a): allow k == a.Maximum tokenization of [0,a) yields a tokens.
- * 2. Let I(a) be the set of all k-token interpretations of substring [0,a), where 1 <= k < a. + * 2. Let I(a) be the set of all k-token interpretations of substring [0,a), where 1 <= k <= a.
19-21: Fix bounds: allow k == n.One-token through n-token interpretations are valid; tighten the bound.
- * Generates all k-token interpretations of the n-character query string, where 1 <= k < n. + * Generates all k-token interpretations of the n-character query string, where 1 <= k <= n.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/log_surgeon/Lexer.hpp(2 hunks)src/log_surgeon/wildcard_query_parser/Query.hpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,h,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/log_surgeon/wildcard_query_parser/Query.hppsrc/log_surgeon/Lexer.hpp
🧠 Learnings (21)
📓 Common learnings
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:57-77
Timestamp: 2025-08-08T10:21:56.571Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp/.cpp (C++), for QueryInterpretation::append_query_interpretation, prefer a single overload taking QueryInterpretation const&; do not suggest adding an rvalue/move overload in future reviews for this method.
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:00:20.963Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp (C++), do not default the comparison operators: the class stores std::vector<std::variant<StaticQueryToken, VariableQueryToken>>, which yields only a weak ordering. A custom operator<=> that maps the variant’s weak ordering to std::strong_ordering is required; avoid suggesting defaulting there in future reviews.
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:17:43.495Z
Learning: In src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp/.cpp (C++), do not suggest defaulting operator<=>. The project prefers a custom out-of-line comparator that explicitly handles the bool member (via explicit cast) to avoid implicit conversions; keep the current manual implementation.
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:135-138
Timestamp: 2025-09-03T16:29:13.677Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.cpp, the is_surrounded_by_delims() method actually considers both delimiters and wildcards as valid boundaries, despite the method name suggesting only delimiters. The implementation uses is_delim_or_wildcard() checks for boundary detection.
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:135-138
Timestamp: 2025-09-03T16:29:13.677Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.cpp, the is_surrounded_by_delims() method actually considers both delimiters and wildcards as valid boundaries, despite the method name suggesting only delimiters. The implementation uses is_delim_or_wildcard() checks for boundary detection.
📚 Learning: 2025-08-08T10:00:20.963Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:00:20.963Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp (C++), do not default the comparison operators: the class stores std::vector<std::variant<StaticQueryToken, VariableQueryToken>>, which yields only a weak ordering. A custom operator<=> that maps the variant’s weak ordering to std::strong_ordering is required; avoid suggesting defaulting there in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:21:56.571Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:57-77
Timestamp: 2025-08-08T10:21:56.571Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp/.cpp (C++), for QueryInterpretation::append_query_interpretation, prefer a single overload taking QueryInterpretation const&; do not suggest adding an rvalue/move overload in future reviews for this method.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:23:06.281Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:80-88
Timestamp: 2025-08-08T10:23:06.281Z
Learning: In y-scope/log-surgeon (C++), small function definitions are allowed to remain inline in headers. For src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp, do not suggest moving small methods like QueryInterpretation::append_variable_token to the .cpp for “consistency” in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:17:43.495Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:17:43.495Z
Learning: In src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp/.cpp (C++), do not suggest defaulting operator<=>. The project prefers a custom out-of-line comparator that explicitly handles the bool member (via explicit cast) to avoid implicit conversions; keep the current manual implementation.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T10:22:26.739Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:98-120
Timestamp: 2025-08-08T10:22:26.739Z
Learning: In C++ file src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp, for QueryInterpretation::serialize(), prefer explicit std::holds_alternative/std::get branching over std::visit for readability; do not suggest refactoring to std::visit in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-09-03T16:45:58.451Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.hpp:119-132
Timestamp: 2025-09-03T16:45:58.451Z
Learning: In log-surgeon's wildcard_query_parser, the DFA intersection process used in Query::get_matching_variable_types loses track of variable type priorities from the original ByteLexer. Priority information cannot be preserved by simply changing the return type from std::set to std::vector - it would require post-processing after the DFA intersection.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-13T12:03:18.145Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:32-37
Timestamp: 2025-08-13T12:03:18.145Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the extend_to_adjacent_greedy_wildcards() method should only extend by one greedy wildcard character on each side, not multiple consecutive ones. This single-wildcard extension is the intended behavior for search operations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-09-03T23:55:09.402Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:95-117
Timestamp: 2025-09-03T23:55:09.402Z
Learning: In src/log_surgeon/wildcard_query_parser/Query.cpp's DP algorithm, extend_to_adjacent_greedy_wildcards() creates a logical extension for interpretation purposes only, while preserving original view boundaries (begin_idx, end_idx) for DP bucket indexing. This allows overlapping segments to be processed correctly in subsequent iterations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-15T00:13:46.717Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: tests/test-wildcard-expression.cpp:0-0
Timestamp: 2025-08-15T00:13:46.717Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpression.cpp, the escaping logic treats consecutive backslashes as escape-literal pairs, not as individual escape characters. For input like R"(a\*b\?c\\)", position 7 is the escape character and position 8 is the literal backslash being escaped, so only position 7 should have is_escape() return true.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-13T12:05:00.245Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:05:00.245Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, the generate_regex_string() method should not enforce well-formedness checks via assertions. The is_well_formed() method is intended to be used by callers at their discretion, allowing flexibility to generate regex strings from malformed views if desired.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-09-03T16:29:13.677Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:135-138
Timestamp: 2025-09-03T16:29:13.677Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.cpp, the is_surrounded_by_delims() method actually considers both delimiters and wildcards as valid boundaries, despite the method name suggesting only delimiters. The implementation uses is_delim_or_wildcard() checks for boundary detection.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T09:31:46.313Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:46.313Z
Learning: In src/log_surgeon/wildcard_query_parser/ExpressionView.hpp, the is_surrounded_by_delims_or_wildcards() method treats escaped characters correctly: escaped wildcards become literal characters (not wildcards), while escaped delimiters still function as delimiters for boundary evaluation. Empty views implicitly don't evaluate first/last characters since they don't exist.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T09:31:28.567Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:46-70
Timestamp: 2025-08-26T09:31:28.567Z
Learning: In src/log_surgeon/wildcard_query_parser, escaped wildcards (\* or \?) lose their wildcard nature entirely and become literal characters, while escaped delimiters retain their delimiter property in the delimiter table regardless of being escaped. This means boundary detection logic correctly treats escaped wildcards as non-wildcards but escaped delimiters as still being delimiters.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-09-03T14:27:18.183Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/ExpressionView.hpp:0-0
Timestamp: 2025-09-03T14:27:18.183Z
Learning: In the log_surgeon wildcard_query_parser module, the correct terminology is: a "wildcard" refers specifically to unescaped * or ? characters with special meaning, while escaped \* or \? characters are just "literal characters" or "literals", not "escaped wildcards". Using "unescaped wildcard" is redundant since wildcards are by definition unescaped.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-08T13:18:39.895Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:42-48
Timestamp: 2025-08-08T13:18:39.895Z
Learning: In y-scope/log-surgeon (C++), it is acceptable/preferred to keep `const` qualifiers on by-value function parameters to signal intent (e.g., in src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp). Do not suggest removing `const` from by-value parameters in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hppsrc/log_surgeon/Lexer.hpp
📚 Learning: 2025-08-13T12:06:36.584Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp:58-77
Timestamp: 2025-08-13T12:06:36.584Z
Learning: In src/log_surgeon/wildcard_query_parser/WildcardExpressionView.cpp, caching SchemaParser::get_special_regex_characters() with a const& reference provides no performance benefit, indicating the method likely returns a reference to an existing container rather than performing expensive computations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T09:51:07.213Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:123-159
Timestamp: 2025-08-26T09:51:07.213Z
Learning: In log-surgeon's wildcard_query_parser, the ByteLexer's get_matching_variable_types method returns variable type IDs already ordered by priority (highest to lowest), so selecting the first ID gives the highest priority match.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-25T20:44:48.955Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.cpp:159-177
Timestamp: 2025-08-25T20:44:48.955Z
Learning: In the log-surgeon codebase, thread safety issues with global state like NonTerminal::m_next_children_start should be addressed comprehensively in dedicated PRs rather than fixed piecemeal in individual feature PRs. The user SharafMohamed prefers to defer such systemic architectural issues to separate PRs.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-26T10:06:22.914Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.hpp:9-11
Timestamp: 2025-08-26T10:06:22.914Z
Learning: In y-scope/log-surgeon project, it's acceptable to include headers like log_surgeon/Lexer.hpp directly in header files rather than using forward declarations, even when only references are used in the interface. The project prefers the simplicity of direct includes over header coupling optimization through forward declarations.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
📚 Learning: 2025-08-15T01:04:03.681Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/ExpressionView.cpp:11-11
Timestamp: 2025-08-15T01:04:03.681Z
Learning: In y-scope/log-surgeon C++ codebase, prefer explicit includes in implementation files (.cpp) for types that are directly used, even when those headers are already transitively included through the corresponding header file. This makes dependencies explicit and improves maintainability.
Applied to files:
src/log_surgeon/wildcard_query_parser/Query.hpp
🧬 Code graph analysis (1)
src/log_surgeon/wildcard_query_parser/Query.hpp (3)
src/log_surgeon/Lexer.hpp (6)
nodiscard(153-153)nodiscard(155-157)nodiscard(159-161)nodiscard(163-166)nodiscard(175-181)nodiscard(188-194)src/log_surgeon/wildcard_query_parser/ExpressionView.hpp (3)
nodiscard(41-44)nodiscard(105-105)nodiscard(108-113)src/log_surgeon/wildcard_query_parser/ExpressionCharacter.hpp (7)
nodiscard(21-21)nodiscard(23-23)nodiscard(25-27)nodiscard(29-31)nodiscard(33-35)nodiscard(37-40)nodiscard(42-42)
⏰ 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). (6)
- GitHub Check: build (ubuntu-24.04, debug)
- GitHub Check: lint (macos-15)
- GitHub Check: lint (ubuntu-24.04)
- GitHub Check: build (macos-15, release)
- GitHub Check: build (ubuntu-22.04, debug)
- GitHub Check: build (macos-15, debug)
🔇 Additional comments (2)
src/log_surgeon/Lexer.hpp (1)
260-262: Docstring for delimiter cache looks good.Clear rationale for caching in the hot path.
src/log_surgeon/wildcard_query_parser/Query.hpp (1)
133-133: Member name aligns with intent.m_processed_query_string matches the API and reduces confusion.
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.
Description
This PR Introduces the
Queryclass for parsing and interpreting queries within Log Surgeon.This
Queryclass converts a query string into a set ofQueryInterpretations by:This resulting set of
QueryInterpretations ensure that any log-event parsed into Log Surgeon's logtype format (using the same schema) can be matched by at least one interpretation.Implementation details
get_matching_variable_types: determines all variable types matching a query substring by intersecting the substring's DFA with the lexer DFA.get_all_single_token_interpretations: generates all single-token interpretations for a given query substring.get_all_multi_token_interpretations: constructs all multi-token interpretations of the full query via dynamic programming.Validation performed
Added unit-tests for the
Queryclass.Summary by CodeRabbit
New Features
Documentation
Tests
Chores