fix(validation): address PR15 Gemini review comments#16
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors imports in status_screen.py, adds a check for empty entries in verify_state_log_chain along with corresponding tests, and updates the JSON schema validator to use re.search instead of re.match for pattern matching (with new tests). The reviewer suggested making the schema pattern validation more robust by ensuring the pattern is a string and handling regular expression compilation errors gracefully.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if "pattern" in schema and isinstance(instance, str): | ||
| import re | ||
| if not re.match(schema["pattern"], instance): | ||
| if not re.search(schema["pattern"], instance): | ||
| raise ValueError(f"{location} does not match pattern {schema['pattern']}") |
There was a problem hiding this comment.
If schema["pattern"] is not a string, or if it is an invalid regular expression, re.search will raise a TypeError or re.error respectively. To make the validator more robust and defensive, we should validate that the pattern is a string and handle any compilation errors gracefully by raising a ValueError.
| if "pattern" in schema and isinstance(instance, str): | |
| import re | |
| if not re.match(schema["pattern"], instance): | |
| if not re.search(schema["pattern"], instance): | |
| raise ValueError(f"{location} does not match pattern {schema['pattern']}") | |
| if "pattern" in schema and isinstance(instance, str): | |
| import re | |
| pattern = schema["pattern"] | |
| if not isinstance(pattern, str): | |
| raise ValueError(f"Schema pattern must be a string, got {type(pattern).__name__}") | |
| try: | |
| compiled_pattern = re.compile(pattern) | |
| except re.error as err: | |
| raise ValueError(f"Invalid regular expression pattern '{pattern}': {err}") | |
| if not compiled_pattern.search(instance): | |
| raise ValueError(f"{location} does not match pattern {pattern}") |
Summary:
Context:
Follow-up to Gemini Code Assist comments on PR #15.
Changed files:
Validation:
Risk:
Low. Small correctness fix plus import cleanup; no public CLI behavior change beyond rejecting invalid empty state log chains.