Enhance template engine with registry integration and improved render…#18
Enhance template engine with registry integration and improved render…#18k-dot-greyz wants to merge 1 commit into
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThis PR introduces a comprehensive template management system with validation, discovery, and CLI utilities, alongside GitHub Actions automation for PR status monitoring. New template registries, schemas, and evolution tracking enable structured template governance, while the PR checking workflow provides automated status reporting. Changes
Sequence Diagram(s)sequenceDiagram
participant GHA as GitHub Actions
participant Script as check_prs.py
participant GHApi as GitHub API
participant PR as Pull Request
GHA->>GHA: Trigger on pull_request/<br/>workflow_dispatch
GHA->>Script: Execute with environment<br/>(GITHUB_TOKEN, args)
Script->>Script: Parse args (--pr, --json,<br/>--conflicts-only)
Script->>GHApi: get_repo().get_pull(pr_num)<br/>or get_pulls(state=open)
GHApi-->>Script: PR object(s)
loop For each PR
Script->>GHApi: Fetch PR metadata<br/>(branches, state, timestamps)
Script->>GHApi: Check mergeable status<br/>& mergeable_state
Script->>GHApi: Get latest commit statuses
Script->>GHApi: Fetch reviews<br/>(approvals, changes requested)
Script->>GHApi: List changed files<br/>(status, additions/deletions)
Script->>Script: Compute ready_to_merge flag<br/>(mergeable + clean + approved)
Script->>Script: Format/aggregate results
end
Script->>Script: Generate summary<br/>(ready, conflicts, drafts)
alt --json flag
Script-->>GHA: Output JSON per-PR
else Default
Script-->>GHA: Pretty-printed report
end
Script->>GHApi: Create/update PR comment<br/>(merge status, draft, state)
GHApi-->>PR: Comment posted/updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| comment.edit(comment) | ||
| break | ||
| else: | ||
| pr.create_issue_comment(comment) |
There was a problem hiding this comment.
Variable shadowing causes incorrect comment editing/creation
High Severity
The variable comment defined as the status message string on line 224 is shadowed by the for loop variable on line 235. This causes two problems: comment.edit(comment) on line 237 passes the comment object to itself instead of the new body text, and in the else branch, pr.create_issue_comment(comment) may use the last comment object from iteration rather than the intended status message string.
Additional Locations (1)
| schema = jsonschema.Draft7Validator.check_schema( | ||
| jsonschema.load(schema_file.open("r", encoding="utf-8")) | ||
| ) | ||
| validator = jsonschema.Draft7Validator(schema) |
There was a problem hiding this comment.
Invalid jsonschema API usage causes runtime failure
High Severity
The code uses jsonschema.load() which doesn't exist in the jsonschema module - json.load() is needed instead. Additionally, Draft7Validator.check_schema() returns None (it validates and raises on error), not the schema itself, so schema will be None and Draft7Validator(schema) will fail. Compare with the correct implementation in dev-master/zen/templates/validator.py which uses json.load() and handles check_schema() properly.
| Template Pokédex package exposing registry-aware utilities. | ||
| """ | ||
|
|
||
| from .pokedex import TemplatePokedex # noqa: F401 |
There was a problem hiding this comment.
Duplicated module docstring and import statements
Low Severity
The module docstring and TemplatePokedex import are duplicated. Lines 6-10 are an exact copy of lines 1-5. The second docstring becomes an orphan string literal and the second import is redundant.
| return ( | ||
| f"Latest version {current_version} introduced breaking changes. " | ||
| "Review dependent automation for compatibility." | ||
| ) |
There was a problem hiding this comment.
Breaking change detection uses wrong nested field path
Medium Severity
The code checks last_entry.get("breaking") to detect breaking changes, but the evolution.yaml structure nests breaking under compatibility (i.e., compatibility.breaking). This means the condition will always be falsy and breaking changes will never be detected. The check needs to access last_entry.get("compatibility", {}).get("breaking") instead.
Additional Locations (1)
| report.add("info", "No schema declared; skipping schema validation.") | ||
| return | ||
|
|
||
| schema_file = self.schema_root.parent / schema_path |
There was a problem hiding this comment.
Schema file path constructed with incorrect parent directory
Medium Severity
The schema file path is constructed as schema_root.parent / schema_path, but schema_root is already templates/metadata/schemas and schema_path from the registry starts with metadata/schemas/.... Using schema_root.parent results in templates/metadata/metadata/schemas/... (doubled metadata/), causing schema files to never be found. Schema validation will always fail with "Schema file missing" warnings.
Additional Locations (1)
| 'unknown': '❓' | ||
| } | ||
| icon = state_icons.get(pr.mergeable_state, '❓') | ||
| print(f" {icon} Merge State: {pr.mergeable_state.upper()}") |
There was a problem hiding this comment.
Missing null check on mergeable_state causes crash
Medium Severity
The code calls .upper() on pr.mergeable_state without checking for None. According to GitHub's API, mergeable_state can be None when GitHub hasn't finished computing the merge status. This will cause an AttributeError crash when the PR's merge state is still being determined. The icon lookup on line 120 handles None correctly with a fallback, but the .upper() call immediately after does not.
Additional Locations (2)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Fix all issues with AI agents
In @.github/workflows/pr-status-check.yml:
- Around line 222-240: The string variable `comment` is being shadowed by the
loop variable `comment`, breaking updates; rename the string to something like
`comment_body` and the loop variable to `existing_comment`, then call the
comment API with the body string (e.g.
`existing_comment.edit(body=comment_body)` or equivalent) and use
`pr.create_issue_comment(comment_body)` when creating a new comment to avoid
passing the wrong object.
In @dev-master/zen/cli.py:
- Around line 119-123: The except block calls an undefined function
parse_variables(value) causing a NameError; fix by either importing or adding a
local function named parse_variables with signature parse_variables(value: str)
-> Dict[str, Any] that parses comma-separated key=value pairs into a dict
(trimming whitespace and handling missing '=' safely), or replace the fallback
return parse_variables(value) in the try/except inside the JSON parsing code
with equivalent inline parsing logic that returns a dict when JSON decoding
fails; ensure the symbol parse_variables is present in the module scope if you
choose the separate function.
In @dev-master/zen/templates/pokedex.py:
- Around line 1-182: Duplicate template registry module detected: remove the
redundant file dev-master/zen/templates/pokedex.py and keep the canonical
zen/templates/pokedex.py; locate the duplicate by searching for the
TemplatePokedex class or TemplateRecord dataclass and delete the entire
duplicate file (dev-master/.../pokedex.py) from the branch, ensuring no other
code references the dev-master path and CI/test imports still point to
zen/templates/pokedex.py.
In @dev-master/zen/templates/validator.py:
- Around line 56-78: The validate method currently uses a truthiness check (if
sample_variables:) which skips render validation when callers intentionally pass
an empty dict {}; change it to an explicit None check (if sample_variables is
not None:) so {} triggers _validate_render(template entry, sample_variables,
report). Do the same replacement for the other occurrence in this file where
sample_variables is checked before calling _validate_render to ensure empty
dicts are validated.
- Around line 103-112: The schema load/validation block currently calls
json.load(schema_file) and jsonschema.Draft7Validator.check_schema(schema)
without handling JSONDecodeError or SchemaError; wrap the json.load(...) and
check_schema(...) calls in a try/except that catches json.JSONDecodeError and
jsonschema.exceptions.SchemaError (and a generic Exception fallback if you
prefer parity with _validate_render), call report.add("error", ...) with a clear
message including the exception message and the schema_file name/path, and abort
further validation (skip creating Draft7Validator/iter_errors) when these
exceptions occur so malformed schema files are reported instead of raising
uncaught exceptions.
- Around line 149-154: The current broad except around
self.engine.render_by_id(entry["id"], variables) should be replaced with a
specific exception tuple: catch TemplateRegistryError, ValueError (and
json.JSONDecodeError if used), and Jinja2 exceptions such as
jinja2.TemplateNotFound, jinja2.UndefinedError (and jinja2.TemplateSyntaxError
if relevant) instead of Exception; update the except clause that currently calls
report.add("error", f"Render failed with sample data: {exc}") to handle only
those exceptions so render failures are logged the same way while letting other
unexpected errors propagate.
In @templates/metadata/evolution.yaml:
- Around line 1-75: The evolution YAML is missing an entry for
metadata.registry.schema which causes TemplatePokedex.get_evolution_history() to
miss that template; add a new evolution block for metadata.registry.schema
mirroring the other entries (set current_version "1.0.0", include a history
array with the initial version, date "2025-11-11", author "human:kaspars", a
brief changes/motivations list, compatibility.breaking false and notes, and a
deprecation block with status "active" and null successor/notes) so the file
lists all templates referenced by templates/registry.yaml and consistent lookups
succeed.
In @templates/metadata/schemas/documents/reports/daily_standup.json:
- Around line 7-27: The schema currently allows non-ISO dates and extra keys in
each update; add a strict format/pattern to the "date" property (e.g., "format":
"date" or a suitable ISO regex) to enforce ISO date strings, and set
"additionalProperties": false on the object schema inside "updates.items" so
unexpected keys in each update are rejected; keep the existing required fields
("member","yesterday","today","blockers") as-is.
In @templates/registry.yaml:
- Around line 1-126: The registry declares five template entries whose path
fields point to missing files; for each id ("doc.report.daily_standup",
"doc.communication.social_post", "automation.n8n.workflow_selector",
"code.python.plugin_boilerplate", "metadata.registry.schema") either create the
referenced template file at the listed path or update the entry's path to the
correct existing file location; ensure the variables_schema references remain
valid (e.g., existing schemas under metadata/schemas/) and keep the id, type and
compatibility fields consistent when changing paths so consumers can resolve
templates reliably.
In @zen/templates/__init__.py:
- Around line 1-11: Remove the duplicated module docstring and duplicated import
of TemplatePokedex in the module and replace the noqa import pattern with an
explicit export list: keep a single top-of-file docstring, a single "from
.pokedex import TemplatePokedex" import, remove the "# noqa: F401" comment, and
add "__all__ = ['TemplatePokedex']" so the package re-exports the symbol cleanly
and avoids lint F811/F401 issues.
In @zen/templates/pokedex.py:
- Around line 130-134: The breaking-change check in the function uses
last_entry.get("breaking") which never matches because the flag lives under
compatibility.breaking in the evolution YAML; update the condition in the block
that returns the breaking-change message (the code referencing last_entry and
current_version) to safely access the nested flag (e.g., inspect
last_entry.get("compatibility", {}).get("breaking") or equivalent null-safe
lookup) so the condition triggers when compatibility.breaking is true.
- Around line 151-162: TemplateRecord.path is declared as a non-optional str but
the return uses entry.get("path") which may be None; either validate/raise or
provide a safe default before constructing the TemplateRecord. Update the code
around the TemplateRecord construction in pokedex.py to read the path value into
a local variable (e.g., path = entry.get("path")), then if path is None either
set path = "" (or a sensible default) or raise a clear ValueError referencing
template_id, or change the TemplateRecord dataclass to make path: Optional[str]
and handle None callers accordingly. Ensure the chosen approach prevents
assigning None to TemplateRecord.path.
In @zen/templates/validator.py:
- Around line 102-109: Replace the incorrect jsonschema calls by loading the
schema dict with Python's json.load(schema_file.open(...)) into a variable named
schema, call jsonschema.Draft7Validator.check_schema(schema) solely to validate
(do not reassign its return value), then instantiate validator =
jsonschema.Draft7Validator(schema) and continue using
validator.iter_errors(variables) as before; ensure you import json if not
present and keep the existing error reporting logic (path construction and
report.add) unchanged.
In @zen/utils/template.py:
- Around line 106-109: The render method currently constructs a standalone
Template (Template(template_str)) which bypasses the Jinja environment and
therefore cannot use custom filters/globals; change it to use the environment's
from_string API (self.env.from_string(template_str)) and call render on that
template so ad-hoc templates inherit filters like "markdown"/"code" and globals
such as "include_template" registered on self.env.
🧹 Nitpick comments (8)
scripts/README_PR_CHECK.md (1)
28-28: Polish documentation formatting.Minor documentation improvements:
- Line 28: Wrap the bare URL in angle brackets or use a Markdown link for better rendering.
- Line 52: Specify a language identifier for the fenced code block (e.g.,
```textor```console).📝 Proposed fixes
-Get a token at: https://github.com/settings/tokens +Get a token at: <https://github.com/settings/tokens>-``` +```text 🔍 Checking all open PRs for k-dot-greyz/zenOS...Also applies to: 52-52
scripts/check_prs.py (1)
104-104: Remove unnecessary f-string prefixes.Several print statements use f-strings without any placeholders. While this doesn't affect functionality, removing the
fprefix improves code clarity.♻️ Proposed fix
# Merge status - print(f"\n📊 Merge Status:") + print("\n📊 Merge Status:") # ... other lines ... # CI status - print(f"\n🔍 CI Status:") + print("\n🔍 CI Status:") # Reviews - print(f"\n👥 Reviews:") + print("\n👥 Reviews:") # Status (lines 140, 142) if pr.draft: - print(f"\n📝 Status: DRAFT (not ready for review)") + print("\n📝 Status: DRAFT (not ready for review)") else: - print(f"\n📝 Status: READY FOR REVIEW") + print("\n📝 Status: READY FOR REVIEW") # Later at line 169 - print(f" 1. git checkout {pr.head.ref}") + print(f" 1. git checkout {pr.head.ref}") # Keep this one, it has a placeholder - print(f" 2. git fetch origin") + print(" 2. git fetch origin")Also applies to: 124-124, 133-133, 140-140, 142-142, 169-169
templates/metadata/schemas/documents/communications/social_post.json (1)
19-26: Considerformat: "uri"forassets[](if these are URLs).Right now any string passes; if the intent is URLs, encoding that improves validation quality.
Optional tweak
"assets": { "type": "array", - "items": {"type": "string"}, + "items": {"type": "string", "format": "uri"}, "description": "Optional list of asset URLs to embed." }templates/metadata/schemas/code/python/plugin_boilerplate.json (1)
7-16:plugin_iddescription says “Snake-case”, but regex allows.and-.If you truly want snake_case only, tighten the pattern; otherwise, tweak the description to match what you’re permitting.
Option A: tighten to snake_case
"plugin_id": { "type": "string", - "pattern": "^[a-z0-9_.-]+$", - "description": "Snake-case identifier used in registry." + "pattern": "^[a-z][a-z0-9_]*$", + "description": "snake_case identifier used in registry." },zen/utils/template.py (1)
141-146: JSON parsing result is discarded.The JSON validation at line 145 parses the rendered content but discards the result. If validation is the only goal, this is acceptable, but consider whether the parsed object should be returned or cached for downstream use.
Also, if
json.loads()fails, theJSONDecodeErrorwill propagate without context about which template failed.♻️ Add context to JSON validation errors
if template_type in {"yaml", "yml", "json"}: content = self._read_template_file(template_path) rendered = self.render(content, variables) if template_type == "json": - json.loads(rendered) # ensure valid JSON after rendering + try: + json.loads(rendered) # ensure valid JSON after rendering + except json.JSONDecodeError as exc: + raise TemplateRegistryError( + f"Template '{template_id}' produced invalid JSON: {exc}" + ) from exc return rendereddev-master/zen/cli.py (2)
67-72:datetime.utcnow()is deprecated in Python 3.12+.Consider using timezone-aware datetime for future compatibility.
♻️ Use timezone-aware datetime
+from datetime import datetime, timezone + def save_registry_data(data: Dict[str, Any]) -> None: - data["last_updated"] = datetime.datetime.utcnow().strftime("%Y-%m-%d") + data["last_updated"] = datetime.now(timezone.utc).strftime("%Y-%m-%d") REGISTRY_PATH.write_text( yaml.safe_dump(data, sort_keys=False, allow_unicode=True), encoding="utf-8", ) def save_evolution_data(data: Dict[str, Any]) -> None: - data["last_updated"] = datetime.datetime.utcnow().strftime("%Y-%m-%d") + data["last_updated"] = datetime.now(timezone.utc).strftime("%Y-%m-%d")Also applies to: 81-86
99-101: Use list unpacking for cleaner code.As flagged by static analysis (RUF005), prefer spread syntax.
♻️ Use spread operator
- components = [prefix] + remainder if remainder else [prefix] + components = [prefix, *remainder] if remainder else [prefix]zen/templates/validator.py (1)
121-125: Accessing private method_read_template_file.The validator accesses
self.engine._read_template_file(path)which is a private method. Consider using the publicread_template_source()method instead for better encapsulation.♻️ Use public API
try: - content = self.engine._read_template_file(path) + content = self.engine.read_template_source(path) except TemplateRegistryError as exc: report.add("error", str(exc)) return
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.cursor/rules/devops.mdc.github/workflows/pr-status-check.ymldev-master/zen/cli.pydev-master/zen/templates/pokedex.pydev-master/zen/templates/validator.pyscripts/README_PR_CHECK.mdscripts/check_prs.pytemplates/metadata/evolution.yamltemplates/metadata/schemas/code/python/plugin_boilerplate.jsontemplates/metadata/schemas/documents/communications/social_post.jsontemplates/metadata/schemas/documents/reports/daily_standup.jsontemplates/registry.yamlzen/templates/__init__.pyzen/templates/pokedex.pyzen/templates/validator.pyzen/utils/template.py
🧰 Additional context used
🧬 Code graph analysis (4)
dev-master/zen/templates/validator.py (1)
zen/utils/template.py (5)
TemplateEngine(19-197)TemplateRegistryError(15-16)get_template_entry(96-101)read_template_source(155-157)render_by_id(120-150)
zen/templates/pokedex.py (2)
zen/utils/template.py (4)
TemplateRegistryError(15-16)refresh_registry(71-73)list_templates(79-94)get_template_entry(96-101)zen/plugins/registry.py (1)
rarity(65-76)
zen/utils/template.py (1)
zen/templates/pokedex.py (2)
exists(79-85)get(74-77)
zen/templates/validator.py (1)
zen/utils/template.py (5)
TemplateEngine(19-197)TemplateRegistryError(15-16)get_template_entry(96-101)_read_template_file(163-168)render_by_id(120-150)
🪛 LanguageTool
scripts/README_PR_CHECK.md
[uncategorized] ~7-~7: The official name of this software platform is spelled with a capital “H”.
Context: ... GitHub Actions Workflow The workflow (.github/workflows/pr-status-check.yml) automat...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
scripts/README_PR_CHECK.md
28-28: Bare URL used
(MD034, no-bare-urls)
52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.10)
dev-master/zen/cli.py
97-97: Avoid specifying long messages outside the exception class
(TRY003)
100-100: Consider [prefix, *remainder] instead of concatenation
Replace with [prefix, *remainder]
(RUF005)
117-117: Avoid specifying long messages outside the exception class
(TRY003)
122-122: Undefined name parse_variables
(F821)
126-126: Consider [*version.split("."), "0", "0"] instead of concatenation
Replace with [*version.split("."), "0", "0"]
(RUF005)
dev-master/zen/templates/validator.py
151-151: Do not catch blind exception: Exception
(BLE001)
scripts/check_prs.py
104-104: f-string without any placeholders
Remove extraneous f prefix
(F541)
124-124: f-string without any placeholders
Remove extraneous f prefix
(F541)
133-133: f-string without any placeholders
Remove extraneous f prefix
(F541)
140-140: f-string without any placeholders
Remove extraneous f prefix
(F541)
142-142: f-string without any placeholders
Remove extraneous f prefix
(F541)
154-154: String contains ambiguous ➕ (HEAVY PLUS SIGN). Did you mean + (PLUS SIGN)?
(RUF001)
156-156: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
169-169: f-string without any placeholders
Remove extraneous f prefix
(F541)
219-219: Do not catch blind exception: Exception
(BLE001)
zen/templates/pokedex.py
56-56: Avoid specifying long messages outside the exception class
(TRY003)
zen/templates/__init__.py
5-5: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
10-10: Redefinition of unused TemplatePokedex from line 5
Remove definition: TemplatePokedex
(F811)
10-10: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
dev-master/zen/templates/pokedex.py
56-56: Avoid specifying long messages outside the exception class
(TRY003)
zen/utils/template.py
62-64: Avoid specifying long messages outside the exception class
(TRY003)
101-101: Avoid specifying long messages outside the exception class
(TRY003)
114-114: Avoid specifying long messages outside the exception class
(TRY003)
131-131: Avoid specifying long messages outside the exception class
(TRY003)
167-167: Avoid specifying long messages outside the exception class
(TRY003)
181-183: Avoid specifying long messages outside the exception class
(TRY003)
zen/templates/validator.py
149-149: Do not catch blind exception: Exception
(BLE001)
⏰ 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). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (10)
.github/workflows/pr-status-check.yml (3)
1-25: LGTM!The workflow configuration is well-structured with appropriate triggers, read-only permissions, and a reasonable timeout.
27-39: LGTM!Setup steps are appropriate with full git history for conflict detection and correct dependencies.
41-203: LGTM!The PR checking logic is comprehensive and handles mergeability, CI status, reviews, and files appropriately.
scripts/check_prs.py (4)
1-24: LGTM!Import structure and error handling for missing dependencies are well implemented.
26-28: LGTM!Token retrieval logic correctly checks both common environment variable names.
31-87: LGTM!Data collection logic is comprehensive and the ready-to-merge criteria are appropriate.
185-265: LGTM!The main function logic is well-structured with proper argument parsing, token validation, and output formatting. The broad exception catch at line 219 is acceptable for a CLI tool to provide user-friendly error messages.
dev-master/zen/templates/validator.py (1)
45-55: I need the review comment content to proceed. Please provide the review comment enclosed in<review_comment>tags that you would like me to verify and rewrite.zen/templates/pokedex.py (1)
164-181: Rarity calculation thresholds differ from plugin registry.The thresholds here (500/100/25/5) differ from
zen/plugins/registry.py(1000/500/100/10). This may be intentional for templates vs plugins, but worth confirming for consistency.zen/templates/validator.py (1)
145-152: Broad exception handling is appropriate here.The broad
except Exceptionat line 149 is acceptable for render validation since template rendering can fail in various ways (Jinja errors, I/O errors, etc.). Thepylint: disablecomment acknowledges this intentional choice.
| # Create status comment | ||
| mergeable_status = "✅ Ready" if pr.mergeable else "❌ Has Conflicts" | ||
| comment = f"""## 🔍 PR Status Check | ||
|
|
||
| **Merge Status:** {mergeable_status} | ||
| **Merge State:** {pr.mergeable_state.upper()} | ||
| **Draft:** {'Yes' if pr.draft else 'No'} | ||
|
|
||
| This PR has been automatically checked. See the Actions tab for full details. | ||
| """ | ||
|
|
||
| # Check if comment already exists | ||
| comments = pr.get_issue_comments() | ||
| for comment in comments: | ||
| if comment.user.login == 'github-actions[bot]' and 'PR Status Check' in comment.body: | ||
| comment.edit(comment) | ||
| break | ||
| else: | ||
| pr.create_issue_comment(comment) |
There was a problem hiding this comment.
Fix variable shadowing bug in comment update logic.
The variable comment is defined at line 224 to hold the comment body string, but is then reused as the loop variable at line 235. This shadows the original string variable, causing:
- Line 237:
comment.edit(comment)passes the comment object instead of the new body string - Line 240:
pr.create_issue_comment(comment)passes the last comment object instead of the body string
This will cause the workflow to fail when attempting to create or update comments.
🐛 Proposed fix
# Create status comment
mergeable_status = "✅ Ready" if pr.mergeable else "❌ Has Conflicts"
- comment = f"""## 🔍 PR Status Check
+ comment_body = f"""## 🔍 PR Status Check
**Merge Status:** {mergeable_status}
**Merge State:** {pr.mergeable_state.upper()}
**Draft:** {'Yes' if pr.draft else 'No'}
This PR has been automatically checked. See the Actions tab for full details.
"""
# Check if comment already exists
comments = pr.get_issue_comments()
+ existing_comment = None
for comment in comments:
if comment.user.login == 'github-actions[bot]' and 'PR Status Check' in comment.body:
- comment.edit(comment)
+ existing_comment = comment
break
- else:
- pr.create_issue_comment(comment)
+
+ if existing_comment:
+ existing_comment.edit(comment_body)
+ else:
+ pr.create_issue_comment(comment_body)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Create status comment | |
| mergeable_status = "✅ Ready" if pr.mergeable else "❌ Has Conflicts" | |
| comment = f"""## 🔍 PR Status Check | |
| **Merge Status:** {mergeable_status} | |
| **Merge State:** {pr.mergeable_state.upper()} | |
| **Draft:** {'Yes' if pr.draft else 'No'} | |
| This PR has been automatically checked. See the Actions tab for full details. | |
| """ | |
| # Check if comment already exists | |
| comments = pr.get_issue_comments() | |
| for comment in comments: | |
| if comment.user.login == 'github-actions[bot]' and 'PR Status Check' in comment.body: | |
| comment.edit(comment) | |
| break | |
| else: | |
| pr.create_issue_comment(comment) | |
| # Create status comment | |
| mergeable_status = "✅ Ready" if pr.mergeable else "❌ Has Conflicts" | |
| comment_body = f"""## 🔍 PR Status Check | |
| **Merge Status:** {mergeable_status} | |
| **Merge State:** {pr.mergeable_state.upper()} | |
| **Draft:** {'Yes' if pr.draft else 'No'} | |
| This PR has been automatically checked. See the Actions tab for full details. | |
| """ | |
| # Check if comment already exists | |
| comments = pr.get_issue_comments() | |
| existing_comment = None | |
| for comment in comments: | |
| if comment.user.login == 'github-actions[bot]' and 'PR Status Check' in comment.body: | |
| existing_comment = comment | |
| break | |
| if existing_comment: | |
| existing_comment.edit(comment_body) | |
| else: | |
| pr.create_issue_comment(comment_body) |
🤖 Prompt for AI Agents
In @.github/workflows/pr-status-check.yml around lines 222 - 240, The string
variable `comment` is being shadowed by the loop variable `comment`, breaking
updates; rename the string to something like `comment_body` and the loop
variable to `existing_comment`, then call the comment API with the body string
(e.g. `existing_comment.edit(body=comment_body)` or equivalent) and use
`pr.create_issue_comment(comment_body)` when creating a new comment to avoid
passing the wrong object.
| try: | ||
| return json.loads(value) | ||
| except json.JSONDecodeError: | ||
| return parse_variables(value) | ||
|
|
There was a problem hiding this comment.
Critical: parse_variables function is undefined.
Line 122 calls parse_variables(value) which is neither defined nor imported in this module. This will cause a NameError at runtime when the fallback path is taken.
🐛 Define or import the missing function
Either define the function locally:
def parse_variables(value: str) -> Dict[str, Any]:
"""Parse key=value pairs from a string."""
result = {}
for pair in value.split(","):
if "=" in pair:
key, val = pair.split("=", 1)
result[key.strip()] = val.strip()
return resultOr fix line 122 to handle the case properly:
try:
return json.loads(value)
except json.JSONDecodeError:
- return parse_variables(value)
+ # Assume key=value format
+ result = {}
+ for pair in value.split(","):
+ if "=" in pair:
+ key, val = pair.split("=", 1)
+ result[key.strip()] = val.strip()
+ return result📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try: | |
| return json.loads(value) | |
| except json.JSONDecodeError: | |
| return parse_variables(value) | |
| try: | |
| return json.loads(value) | |
| except json.JSONDecodeError: | |
| # Assume key=value format | |
| result = {} | |
| for pair in value.split(","): | |
| if "=" in pair: | |
| key, val = pair.split("=", 1) | |
| result[key.strip()] = val.strip() | |
| return result |
🧰 Tools
🪛 Ruff (0.14.10)
122-122: Undefined name parse_variables
(F821)
🤖 Prompt for AI Agents
In @dev-master/zen/cli.py around lines 119 - 123, The except block calls an
undefined function parse_variables(value) causing a NameError; fix by either
importing or adding a local function named parse_variables with signature
parse_variables(value: str) -> Dict[str, Any] that parses comma-separated
key=value pairs into a dict (trimming whitespace and handling missing '='
safely), or replace the fallback return parse_variables(value) in the try/except
inside the JSON parsing code with equivalent inline parsing logic that returns a
dict when JSON decoding fails; ensure the symbol parse_variables is present in
the module scope if you choose the separate function.
| """ | ||
| Template Pokédex - discovery, cataloging, and analytics for the template registry. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from dataclasses import dataclass | ||
| from pathlib import Path | ||
| from typing import Any, Dict, Iterable, List, Optional | ||
|
|
||
| import yaml | ||
|
|
||
| from zen.utils.template import TemplateEngine, TemplateRegistryError | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class TemplateRecord: | ||
| """Normalized representation of a template registry entry.""" | ||
|
|
||
| id: str | ||
| name: str | ||
| path: str | ||
| type: str | ||
| tags: List[str] | ||
| version: str | ||
| rarity: str | ||
| usage_count: int | ||
| metadata: Dict[str, Any] | ||
| evolution: Dict[str, Any] | ||
|
|
||
|
|
||
| class TemplatePokedex: | ||
| """ | ||
| High-level facade around TemplateEngine providing template discovery, | ||
| rarity calculation, evolution tracking, and usage analytics. | ||
| """ | ||
|
|
||
| def __init__( | ||
| self, | ||
| engine: Optional[TemplateEngine] = None, | ||
| evolution_path: Optional[Path] = None, | ||
| ): | ||
| self.engine = engine or TemplateEngine() | ||
| repo_root = Path(__file__).resolve().parents[1] | ||
| if evolution_path is None: | ||
| evolution_path = repo_root.parent / "templates" / "metadata" / "evolution.yaml" | ||
|
|
||
| self.evolution_path = evolution_path | ||
| self._evolution_index = self._load_evolution(evolution_path) | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # Loading and indexing | ||
| # ------------------------------------------------------------------ | ||
| def _load_evolution(self, path: Path) -> Dict[str, Dict[str, Any]]: | ||
| if not path.exists(): | ||
| raise TemplateRegistryError(f"Evolution tracker not found at {path}") | ||
|
|
||
| data = yaml.safe_load(path.read_text(encoding="utf-8")) or {} | ||
| return data.get("lineage", {}) | ||
|
|
||
| def refresh(self) -> None: | ||
| """Refresh registry and evolution metadata.""" | ||
| self.engine.refresh_registry() | ||
| self._evolution_index = self._load_evolution(self.evolution_path) | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # Catalog operations | ||
| # ------------------------------------------------------------------ | ||
| def catalog(self, *, tags: Optional[Iterable[str]] = None) -> List[TemplateRecord]: | ||
| """Return catalog of templates optionally filtered by tags.""" | ||
| entries = self.engine.list_templates(tags=tags) | ||
| return [self._to_record(entry) for entry in entries] | ||
|
|
||
| def get(self, template_id: str) -> TemplateRecord: | ||
| """Retrieve a single template record by id.""" | ||
| entry = self.engine.get_template_entry(template_id) | ||
| return self._to_record(entry) | ||
|
|
||
| def exists(self, template_id: str) -> bool: | ||
| """Check if template id exists in the registry.""" | ||
| try: | ||
| self.engine.get_template_entry(template_id) | ||
| except TemplateRegistryError: | ||
| return False | ||
| return True | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # Analytics and evolution | ||
| # ------------------------------------------------------------------ | ||
| def get_usage_summary(self, template_id: str) -> Dict[str, Any]: | ||
| """Return usage analytics for the template.""" | ||
| record = self.get(template_id) | ||
| usage = record.metadata.get("usage", {}) | ||
| return { | ||
| "usage_count": usage.get("usage_count", 0), | ||
| "last_used": usage.get("last_used"), | ||
| "average_render_time_ms": usage.get("average_render_time_ms"), | ||
| "rarity": record.rarity, | ||
| "recommended_evolution": self.suggest_evolution(template_id), | ||
| } | ||
|
|
||
| def get_evolution_history(self, template_id: str) -> Dict[str, Any]: | ||
| """Return evolution history for a template.""" | ||
| return self._evolution_index.get(template_id, {}) | ||
|
|
||
| def suggest_evolution(self, template_id: str) -> Optional[str]: | ||
| """ | ||
| Provide evolution suggestion based on usage and lineage. | ||
|
|
||
| Returns human-readable suggestion string or None. | ||
| """ | ||
| record = self.get(template_id) | ||
| usage = record.metadata.get("usage", {}) | ||
| evolution_meta = self.get_evolution_history(template_id) | ||
|
|
||
| if not evolution_meta: | ||
| return None | ||
|
|
||
| current_version = evolution_meta.get("current_version") | ||
| history = evolution_meta.get("history", []) | ||
| last_entry = history[-1] if history else None | ||
| usage_count = usage.get("usage_count", 0) | ||
|
|
||
| if usage_count > 100 and record.rarity in {"common", "uncommon"}: | ||
| return ( | ||
| f"High usage detected ({usage_count}). " | ||
| f"Consider evolving '{template_id}' to incorporate advanced variants." | ||
| ) | ||
|
|
||
| if last_entry and last_entry.get("breaking"): | ||
| return ( | ||
| f"Latest version {current_version} introduced breaking changes. " | ||
| "Review dependent automation for compatibility." | ||
| ) | ||
|
|
||
| successor = evolution_meta.get("deprecation", {}).get("successor") | ||
| if successor: | ||
| return f"Template deprecated. Migrate usage to successor '{successor}'." | ||
|
|
||
| return None | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # Internal helpers | ||
| # ------------------------------------------------------------------ | ||
| def _to_record(self, entry: Dict[str, Any]) -> TemplateRecord: | ||
| template_id = entry["id"] | ||
| evolution = self._evolution_index.get(template_id, {}) | ||
| usage = entry.get("usage", {}) | ||
| rarity = self._calculate_rarity(entry, usage) | ||
|
|
||
| return TemplateRecord( | ||
| id=template_id, | ||
| name=entry.get("name", template_id), | ||
| path=entry.get("path"), | ||
| type=entry.get("type", "text"), | ||
| tags=list(entry.get("tags", [])), | ||
| version=entry.get("version", "0.0.0"), | ||
| rarity=rarity, | ||
| usage_count=usage.get("usage_count", 0), | ||
| metadata=entry, | ||
| evolution=evolution, | ||
| ) | ||
|
|
||
| def _calculate_rarity(self, entry: Dict[str, Any], usage: Dict[str, Any]) -> str: | ||
| """Calculate rarity dynamically using usage, versioning, and tags.""" | ||
| base_rarity = entry.get("rarity") | ||
| if base_rarity: | ||
| return base_rarity | ||
|
|
||
| usage_count = usage.get("usage_count", 0) | ||
| tags = set(entry.get("tags", [])) | ||
|
|
||
| if usage_count > 500 or "critical" in tags: | ||
| return "legendary" | ||
| if usage_count > 100 or "automation" in tags: | ||
| return "epic" | ||
| if usage_count > 25 or "ai-generated" in tags: | ||
| return "rare" | ||
| if usage_count > 5: | ||
| return "uncommon" | ||
| return "common" | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all pokedex.py files in the repository
echo "=== Searching for pokedex.py files ==="
fd -t f "pokedex.py"Repository: k-dot-greyz/zenOS
Length of output: 175
🏁 Script executed:
#!/bin/bash
# Get file sizes and compare contents
echo "=== File sizes ==="
wc -l dev-master/zen/templates/pokedex.py zen/templates/pokedex.py zen/pokedex.py
echo ""
echo "=== Comparing dev-master/zen/templates/pokedex.py vs zen/templates/pokedex.py ==="
diff dev-master/zen/templates/pokedex.py zen/templates/pokedex.py > /tmp/diff1.txt 2>&1
if [ $? -eq 0 ]; then
echo "Files are IDENTICAL"
else
echo "Files DIFFER. First 30 lines of diff:"
head -30 /tmp/diff1.txt
fi
echo ""
echo "=== Comparing zen/templates/pokedex.py vs zen/pokedex.py ==="
diff zen/templates/pokedex.py zen/pokedex.py > /tmp/diff2.txt 2>&1
if [ $? -eq 0 ]; then
echo "Files are IDENTICAL"
else
echo "Files DIFFER. First 30 lines of diff:"
head -30 /tmp/diff2.txt
fi
echo ""
echo "=== Comparing dev-master/zen/templates/pokedex.py vs zen/pokedex.py ==="
diff dev-master/zen/templates/pokedex.py zen/pokedex.py > /tmp/diff3.txt 2>&1
if [ $? -eq 0 ]; then
echo "Files are IDENTICAL"
else
echo "Files DIFFER. First 30 lines of diff:"
head -30 /tmp/diff3.txt
fiRepository: k-dot-greyz/zenOS
Length of output: 1893
🏁 Script executed:
#!/bin/bash
# View the zen/pokedex.py file to see what's different
echo "=== Contents of zen/pokedex.py ==="
cat zen/pokedex.pyRepository: k-dot-greyz/zenOS
Length of output: 7832
Remove duplicate: dev-master/zen/templates/pokedex.py
This file is identical to zen/templates/pokedex.py. The dev-master/ prefix directory contains a redundant copy that should be deleted. Keep only zen/templates/pokedex.py as the canonical location.
Note: There is a separate, unrelated zen/pokedex.py module (for model/procedure catalog) that differs in purpose and should not be confused with this template registry module.
🧰 Tools
🪛 Ruff (0.14.10)
56-56: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In @dev-master/zen/templates/pokedex.py around lines 1 - 182, Duplicate template
registry module detected: remove the redundant file
dev-master/zen/templates/pokedex.py and keep the canonical
zen/templates/pokedex.py; locate the duplicate by searching for the
TemplatePokedex class or TemplateRecord dataclass and delete the entire
duplicate file (dev-master/.../pokedex.py) from the branch, ensuring no other
code references the dev-master path and CI/test imports still point to
zen/templates/pokedex.py.
| def validate(self, template_id: str, sample_variables: Optional[Dict[str, Any]] = None) -> ValidationReport: | ||
| """ | ||
| Validate template correctness using registry metadata. | ||
|
|
||
| Args: | ||
| template_id: Registry id of the template to validate. | ||
| sample_variables: Optional sample payload for render validation. | ||
| """ | ||
| report = ValidationReport(template_id=template_id, is_valid=True) | ||
|
|
||
| try: | ||
| entry = self.engine.get_template_entry(template_id) | ||
| except TemplateRegistryError as exc: | ||
| report.add("error", str(exc)) | ||
| return report | ||
|
|
||
| self._validate_schema(entry, sample_variables or {}, report) | ||
| self._lint_placeholders(entry, report) | ||
| self._lint_metadata(entry, report) | ||
|
|
||
| if sample_variables: | ||
| self._validate_render(entry, sample_variables, report) | ||
|
|
There was a problem hiding this comment.
sample_variables truthiness check skips render validation for {}.
If a caller passes {} intentionally (to verify “missing required vars” behavior), if sample_variables: will skip _validate_render. Consider if sample_variables is not None:.
Proposed fix
- if sample_variables:
+ if sample_variables is not None:
self._validate_render(entry, sample_variables, report)Also applies to: 147-154
🤖 Prompt for AI Agents
In @dev-master/zen/templates/validator.py around lines 56 - 78, The validate
method currently uses a truthiness check (if sample_variables:) which skips
render validation when callers intentionally pass an empty dict {}; change it to
an explicit None check (if sample_variables is not None:) so {} triggers
_validate_render(template entry, sample_variables, report). Do the same
replacement for the other occurrence in this file where sample_variables is
checked before calling _validate_render to ensure empty dicts are validated.
| with schema_file.open("r", encoding="utf-8") as handle: | ||
| schema = json.load(handle) | ||
|
|
||
| jsonschema.Draft7Validator.check_schema(schema) | ||
| validator = jsonschema.Draft7Validator(schema) | ||
| errors = sorted(validator.iter_errors(variables), key=lambda e: e.path) | ||
| for error in errors: | ||
| path = ".".join(map(str, error.path)) or "<root>" | ||
| report.add("error", f"Schema violation at {path}: {error.message}") | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "validator.py" -type f | head -20Repository: k-dot-greyz/zenOS
Length of output: 129
🏁 Script executed:
# Find the file in the repository
git ls-files | grep -i validator.pyRepository: k-dot-greyz/zenOS
Length of output: 125
🏁 Script executed:
# Get the size and content of the validator.py file
wc -l dev-master/zen/templates/validator.py 2>/dev/null || wc -l zen/templates/validator.py 2>/dev/null || find . -name "validator.py" -type f -exec wc -l {} \;Repository: k-dot-greyz/zenOS
Length of output: 102
🏁 Script executed:
cat -n dev-master/zen/templates/validator.pyRepository: k-dot-greyz/zenOS
Length of output: 6817
🏁 Script executed:
# Check if there's any exception handling around the schema validation
# or if jsonschema is used elsewhere in the codebase
rg -A 5 -B 5 "check_schema\|jsonschema" dev-master/zen/templates/validator.pyRepository: k-dot-greyz/zenOS
Length of output: 43
🏁 Script executed:
# Check if there are any tests that might show expected behavior
find . -name "*test*validator*" -o -name "*validator*test*" | grep -E "\.(py|json)$"Repository: k-dot-greyz/zenOS
Length of output: 43
🏁 Script executed:
# Search for any exception handling in the validator or related code
rg "except|try" dev-master/zen/templates/validator.pyRepository: k-dot-greyz/zenOS
Length of output: 1741
🏁 Script executed:
# Check imports and jsonschema usage patterns
head -20 dev-master/zen/templates/validator.py | tail -10Repository: k-dot-greyz/zenOS
Length of output: 292
🏁 Script executed:
# Let's verify the exceptions that jsonschema can raise
python3 << 'EOF'
try:
import jsonschema
# Check what exceptions jsonschema defines
print("jsonschema exceptions:")
print("- SchemaError:", hasattr(jsonschema, 'SchemaError'))
print("- ValidationError:", hasattr(jsonschema, 'ValidationError'))
# Test check_schema with invalid schema
try:
jsonschema.Draft7Validator.check_schema({"type": "invalid_type"})
print("\nInvalid schema didn't raise exception")
except Exception as e:
print(f"\nInvalid schema raised: {type(e).__name__}: {e}")
# Test with malformed schema
try:
jsonschema.Draft7Validator.check_schema({123: "not_a_string_key"})
print("Malformed schema didn't raise exception")
except Exception as e:
print(f"Malformed schema raised: {type(e).__name__}: {e}")
except ImportError:
print("jsonschema not installed")
EOFRepository: k-dot-greyz/zenOS
Length of output: 147
🏁 Script executed:
# Search for tests related to validator
find . -type f -name "*.py" | xargs grep -l "TemplateValidator\|validator\.py" | head -10Repository: k-dot-greyz/zenOS
Length of output: 153
🏁 Script executed:
# Check if there are JSON schema files that might be tested
find . -type f -name "*.json" | grep -i schema | head -10Repository: k-dot-greyz/zenOS
Length of output: 262
🏁 Script executed:
# Check the actual behavior of json.load and error.path
python3 << 'EOF'
import json
from pathlib import Path
from io import StringIO
# Test 1: json.load with invalid JSON
print("Test 1: json.load with invalid JSON")
try:
data = json.loads("{invalid json}")
except Exception as e:
print(f" Raises {type(e).__name__}: {str(e)[:60]}")
# Test 2: Check error.path type from jsonschema
print("\nTest 2: Checking error.path behavior")
print(" error.path is a deque, can be iterated with map(str, error.path)")
print(" Converting to list(error.path) is valid but not strictly necessary for sorting")
# Test 3: Schema validation with jsonschema
print("\nTest 3: jsonschema.Draft7Validator.check_schema behavior")
print(" - Raises SchemaError if schema is invalid per Draft7")
print(" - This can happen with malformed schemas")
EOFRepository: k-dot-greyz/zenOS
Length of output: 525
🏁 Script executed:
# Check the CLI usage to see if there's outer exception handling
rg -A 10 -B 5 "TemplateValidator\|_validate_schema" dev-master/zen/cli.py | head -40Repository: k-dot-greyz/zenOS
Length of output: 43
🏁 Script executed:
# Check if there are any test files that show expected behavior
find . -path "*/test*" -name "*.py" -type f | head -5Repository: k-dot-greyz/zenOS
Length of output: 171
🏁 Script executed:
# Check test files for validator tests
rg -l "validator" *.py test*.py 2>/dev/null | head -5Repository: k-dot-greyz/zenOS
Length of output: 43
🏁 Script executed:
# Look at the test_runner or other tests to understand the pattern
head -50 test_runner.pyRepository: k-dot-greyz/zenOS
Length of output: 1309
🏁 Script executed:
# Check if _validate_schema is called in a way that might already handle exceptions
rg -B 5 -A 5 "_validate_schema" dev-master/zen/templates/validator.pyRepository: k-dot-greyz/zenOS
Length of output: 1088
Add exception handling for schema load and validation failures.
Lines 103-106 lack exception handling for json.load() (raises JSONDecodeError) and check_schema() (raises SchemaError). Invalid or malformed schema files will propagate uncaught exceptions instead of being reported as validation errors, inconsistent with the _validate_render() method which properly catches and reports exceptions.
Proposed fix
with schema_file.open("r", encoding="utf-8") as handle:
schema = json.load(handle)
- jsonschema.Draft7Validator.check_schema(schema)
- validator = jsonschema.Draft7Validator(schema)
- errors = sorted(validator.iter_errors(variables), key=lambda e: e.path)
+ try:
+ jsonschema.Draft7Validator.check_schema(schema)
+ validator = jsonschema.Draft7Validator(schema)
+ errors = sorted(validator.iter_errors(variables), key=lambda e: list(e.path))
+ except Exception as exc: # jsonschema SchemaError / JSONDecodeError / etc.
+ report.add("error", f"Schema validation setup failed: {exc}")
+ return📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| with schema_file.open("r", encoding="utf-8") as handle: | |
| schema = json.load(handle) | |
| jsonschema.Draft7Validator.check_schema(schema) | |
| validator = jsonschema.Draft7Validator(schema) | |
| errors = sorted(validator.iter_errors(variables), key=lambda e: e.path) | |
| for error in errors: | |
| path = ".".join(map(str, error.path)) or "<root>" | |
| report.add("error", f"Schema violation at {path}: {error.message}") | |
| with schema_file.open("r", encoding="utf-8") as handle: | |
| schema = json.load(handle) | |
| try: | |
| jsonschema.Draft7Validator.check_schema(schema) | |
| validator = jsonschema.Draft7Validator(schema) | |
| errors = sorted(validator.iter_errors(variables), key=lambda e: list(e.path)) | |
| except Exception as exc: # jsonschema SchemaError / JSONDecodeError / etc. | |
| report.add("error", f"Schema validation setup failed: {exc}") | |
| return | |
| for error in errors: | |
| path = ".".join(map(str, error.path)) or "<root>" | |
| report.add("error", f"Schema violation at {path}: {error.message}") |
🤖 Prompt for AI Agents
In @dev-master/zen/templates/validator.py around lines 103 - 112, The schema
load/validation block currently calls json.load(schema_file) and
jsonschema.Draft7Validator.check_schema(schema) without handling JSONDecodeError
or SchemaError; wrap the json.load(...) and check_schema(...) calls in a
try/except that catches json.JSONDecodeError and
jsonschema.exceptions.SchemaError (and a generic Exception fallback if you
prefer parity with _validate_render), call report.add("error", ...) with a clear
message including the exception message and the schema_file name/path, and abort
further validation (skip creating Draft7Validator/iter_errors) when these
exceptions occur so malformed schema files are reported instead of raising
uncaught exceptions.
| if last_entry and last_entry.get("breaking"): | ||
| return ( | ||
| f"Latest version {current_version} introduced breaking changes. " | ||
| "Review dependent automation for compatibility." | ||
| ) |
There was a problem hiding this comment.
Breaking change detection uses wrong path.
The code checks last_entry.get("breaking"), but in evolution.yaml, the breaking flag is nested under compatibility.breaking. This condition will never trigger.
🐛 Fix nested path access
- if last_entry and last_entry.get("breaking"):
+ if last_entry and last_entry.get("compatibility", {}).get("breaking"):
return (
f"Latest version {current_version} introduced breaking changes. "
"Review dependent automation for compatibility."
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if last_entry and last_entry.get("breaking"): | |
| return ( | |
| f"Latest version {current_version} introduced breaking changes. " | |
| "Review dependent automation for compatibility." | |
| ) | |
| if last_entry and last_entry.get("compatibility", {}).get("breaking"): | |
| return ( | |
| f"Latest version {current_version} introduced breaking changes. " | |
| "Review dependent automation for compatibility." | |
| ) |
🤖 Prompt for AI Agents
In @zen/templates/pokedex.py around lines 130 - 134, The breaking-change check
in the function uses last_entry.get("breaking") which never matches because the
flag lives under compatibility.breaking in the evolution YAML; update the
condition in the block that returns the breaking-change message (the code
referencing last_entry and current_version) to safely access the nested flag
(e.g., inspect last_entry.get("compatibility", {}).get("breaking") or equivalent
null-safe lookup) so the condition triggers when compatibility.breaking is true.
| return TemplateRecord( | ||
| id=template_id, | ||
| name=entry.get("name", template_id), | ||
| path=entry.get("path"), | ||
| type=entry.get("type", "text"), | ||
| tags=list(entry.get("tags", [])), | ||
| version=entry.get("version", "0.0.0"), | ||
| rarity=rarity, | ||
| usage_count=usage.get("usage_count", 0), | ||
| metadata=entry, | ||
| evolution=evolution, | ||
| ) |
There was a problem hiding this comment.
Potential None assignment to non-optional path field.
TemplateRecord.path is typed as str, but entry.get("path") can return None if the registry entry is malformed. This would violate the dataclass type contract.
♻️ Provide default or validate
return TemplateRecord(
id=template_id,
name=entry.get("name", template_id),
- path=entry.get("path"),
+ path=entry.get("path", ""),
type=entry.get("type", "text"),
tags=list(entry.get("tags", [])),
version=entry.get("version", "0.0.0"),Or update the dataclass to use Optional[str]:
@dataclass(frozen=True)
class TemplateRecord:
"""Normalized representation of a template registry entry."""
id: str
name: str
- path: str
+ path: Optional[str]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return TemplateRecord( | |
| id=template_id, | |
| name=entry.get("name", template_id), | |
| path=entry.get("path"), | |
| type=entry.get("type", "text"), | |
| tags=list(entry.get("tags", [])), | |
| version=entry.get("version", "0.0.0"), | |
| rarity=rarity, | |
| usage_count=usage.get("usage_count", 0), | |
| metadata=entry, | |
| evolution=evolution, | |
| ) | |
| return TemplateRecord( | |
| id=template_id, | |
| name=entry.get("name", template_id), | |
| path=entry.get("path", ""), | |
| type=entry.get("type", "text"), | |
| tags=list(entry.get("tags", [])), | |
| version=entry.get("version", "0.0.0"), | |
| rarity=rarity, | |
| usage_count=usage.get("usage_count", 0), | |
| metadata=entry, | |
| evolution=evolution, | |
| ) |
🤖 Prompt for AI Agents
In @zen/templates/pokedex.py around lines 151 - 162, TemplateRecord.path is
declared as a non-optional str but the return uses entry.get("path") which may
be None; either validate/raise or provide a safe default before constructing the
TemplateRecord. Update the code around the TemplateRecord construction in
pokedex.py to read the path value into a local variable (e.g., path =
entry.get("path")), then if path is None either set path = "" (or a sensible
default) or raise a clear ValueError referencing template_id, or change the
TemplateRecord dataclass to make path: Optional[str] and handle None callers
accordingly. Ensure the chosen approach prevents assigning None to
TemplateRecord.path.
| schema = jsonschema.Draft7Validator.check_schema( | ||
| jsonschema.load(schema_file.open("r", encoding="utf-8")) | ||
| ) | ||
| validator = jsonschema.Draft7Validator(schema) | ||
| errors = sorted(validator.iter_errors(variables), key=lambda e: e.path) | ||
| for error in errors: | ||
| path = ".".join(map(str, error.path)) or "<root>" | ||
| report.add("error", f"Schema violation at {path}: {error.message}") |
There was a problem hiding this comment.
Critical: Incorrect jsonschema API usage.
The code has multiple issues:
jsonschema.load()doesn't exist - usejson.load()to read the schema fileDraft7Validator.check_schema()returnsNone(it validates the schema structure) - it shouldn't be assigned toschema- The validator should be created with the loaded schema dictionary
🐛 Fix jsonschema usage
+import json
+
# ... in _validate_schema method:
- schema = jsonschema.Draft7Validator.check_schema(
- jsonschema.load(schema_file.open("r", encoding="utf-8"))
- )
- validator = jsonschema.Draft7Validator(schema)
+ with schema_file.open("r", encoding="utf-8") as f:
+ schema = json.load(f)
+ try:
+ jsonschema.Draft7Validator.check_schema(schema)
+ except jsonschema.SchemaError as exc:
+ report.add("error", f"Invalid schema: {exc.message}")
+ return
+ validator = jsonschema.Draft7Validator(schema)
errors = sorted(validator.iter_errors(variables), key=lambda e: e.path)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| schema = jsonschema.Draft7Validator.check_schema( | |
| jsonschema.load(schema_file.open("r", encoding="utf-8")) | |
| ) | |
| validator = jsonschema.Draft7Validator(schema) | |
| errors = sorted(validator.iter_errors(variables), key=lambda e: e.path) | |
| for error in errors: | |
| path = ".".join(map(str, error.path)) or "<root>" | |
| report.add("error", f"Schema violation at {path}: {error.message}") | |
| with schema_file.open("r", encoding="utf-8") as f: | |
| schema = json.load(f) | |
| try: | |
| jsonschema.Draft7Validator.check_schema(schema) | |
| except jsonschema.SchemaError as exc: | |
| report.add("error", f"Invalid schema: {exc.message}") | |
| return | |
| validator = jsonschema.Draft7Validator(schema) | |
| errors = sorted(validator.iter_errors(variables), key=lambda e: e.path) | |
| for error in errors: | |
| path = ".".join(map(str, error.path)) or "<root>" | |
| report.add("error", f"Schema violation at {path}: {error.message}") |
🤖 Prompt for AI Agents
In @zen/templates/validator.py around lines 102 - 109, Replace the incorrect
jsonschema calls by loading the schema dict with Python's
json.load(schema_file.open(...)) into a variable named schema, call
jsonschema.Draft7Validator.check_schema(schema) solely to validate (do not
reassign its return value), then instantiate validator =
jsonschema.Draft7Validator(schema) and continue using
validator.iter_errors(variables) as before; ensure you import json if not
present and keep the existing error reporting logic (path construction and
report.add) unchanged.
| def render(self, template_str: str, variables: Dict[str, Any]) -> str: | ||
| """Render an ad-hoc template string with variables.""" | ||
| template = Template(template_str) | ||
| return template.render(**variables) |
There was a problem hiding this comment.
Ad-hoc templates bypass Jinja environment filters and globals.
The render method creates a standalone Template object that doesn't have access to the custom filters (markdown, code) or globals (include_template) registered on self.env. If ad-hoc templates use these features, they will fail.
♻️ Use environment's from_string instead
def render(self, template_str: str, variables: Dict[str, Any]) -> str:
"""Render an ad-hoc template string with variables."""
- template = Template(template_str)
+ template = self.env.from_string(template_str)
return template.render(**variables)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def render(self, template_str: str, variables: Dict[str, Any]) -> str: | |
| """Render an ad-hoc template string with variables.""" | |
| template = Template(template_str) | |
| return template.render(**variables) | |
| def render(self, template_str: str, variables: Dict[str, Any]) -> str: | |
| """Render an ad-hoc template string with variables.""" | |
| template = self.env.from_string(template_str) | |
| return template.render(**variables) |
🤖 Prompt for AI Agents
In @zen/utils/template.py around lines 106 - 109, The render method currently
constructs a standalone Template (Template(template_str)) which bypasses the
Jinja environment and therefore cannot use custom filters/globals; change it to
use the environment's from_string API (self.env.from_string(template_str)) and
call render on that template so ad-hoc templates inherit filters like
"markdown"/"code" and globals such as "include_template" registered on self.env.
| def _validate_variables(self, entry: Dict[str, Any], variables: Dict[str, Any]) -> None: | ||
| """ | ||
| Basic variable validation using registry metadata. | ||
|
|
||
| - Ensures required variables are present | ||
| - Provides hints for missing optional blocks | ||
| """ | ||
| requirements = entry.get("requirements", {}) | ||
| required_vars = requirements.get("required_variables", []) | ||
| missing = [var for var in required_vars if var not in variables] | ||
| if missing: | ||
| raise TemplateRegistryError( | ||
| f"Template '{entry['id']}' missing required variables: {', '.join(missing)}" | ||
| ) | ||
|
|
||
| # Optional schema reference recorded for downstream validation | ||
| schema_path = entry.get("compatibility", {}).get("variables_schema") | ||
| if schema_path and not (self.template_dir / schema_path).exists(): | ||
| # Defer schema enforcement to validator subsystem; warn in debug logs | ||
| pass |
There was a problem hiding this comment.
Variable validation references non-existent registry field.
The _validate_variables method looks for entry.get("requirements", {}).get("required_variables", []), but the registry entries in templates/registry.yaml don't define a requirements field. This means variable validation will never enforce any constraints.
Consider either:
- Adding a
requirementsfield to registry entries that need variable validation - Deriving required variables from the schema referenced in
compatibility.variables_schema
💡 Alternative: Load required variables from schema
def _validate_variables(self, entry: Dict[str, Any], variables: Dict[str, Any]) -> None:
"""
Basic variable validation using registry metadata.
- Ensures required variables are present
- Provides hints for missing optional blocks
"""
- requirements = entry.get("requirements", {})
- required_vars = requirements.get("required_variables", [])
+ # Try to infer required variables from schema if available
+ schema_path = entry.get("compatibility", {}).get("variables_schema")
+ required_vars = []
+ if schema_path:
+ schema_file = self.template_dir / schema_path
+ if schema_file.exists():
+ import json
+ schema = json.loads(schema_file.read_text(encoding="utf-8"))
+ required_vars = schema.get("required", [])
+
missing = [var for var in required_vars if var not in variables]
if missing:
raise TemplateRegistryError(
f"Template '{entry['id']}' missing required variables: {', '.join(missing)}"
)
-
- # Optional schema reference recorded for downstream validation
- schema_path = entry.get("compatibility", {}).get("variables_schema")
- if schema_path and not (self.template_dir / schema_path).exists():
- # Defer schema enforcement to validator subsystem; warn in debug logs
- pass📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _validate_variables(self, entry: Dict[str, Any], variables: Dict[str, Any]) -> None: | |
| """ | |
| Basic variable validation using registry metadata. | |
| - Ensures required variables are present | |
| - Provides hints for missing optional blocks | |
| """ | |
| requirements = entry.get("requirements", {}) | |
| required_vars = requirements.get("required_variables", []) | |
| missing = [var for var in required_vars if var not in variables] | |
| if missing: | |
| raise TemplateRegistryError( | |
| f"Template '{entry['id']}' missing required variables: {', '.join(missing)}" | |
| ) | |
| # Optional schema reference recorded for downstream validation | |
| schema_path = entry.get("compatibility", {}).get("variables_schema") | |
| if schema_path and not (self.template_dir / schema_path).exists(): | |
| # Defer schema enforcement to validator subsystem; warn in debug logs | |
| pass | |
| def _validate_variables(self, entry: Dict[str, Any], variables: Dict[str, Any]) -> None: | |
| """ | |
| Basic variable validation using registry metadata. | |
| - Ensures required variables are present | |
| - Provides hints for missing optional blocks | |
| """ | |
| # Try to infer required variables from schema if available | |
| schema_path = entry.get("compatibility", {}).get("variables_schema") | |
| required_vars = [] | |
| if schema_path: | |
| schema_file = self.template_dir / schema_path | |
| if schema_file.exists(): | |
| import json | |
| schema = json.loads(schema_file.read_text(encoding="utf-8")) | |
| required_vars = schema.get("required", []) | |
| missing = [var for var in required_vars if var not in variables] | |
| if missing: | |
| raise TemplateRegistryError( | |
| f"Template '{entry['id']}' missing required variables: {', '.join(missing)}" | |
| ) |
🧰 Tools
🪛 Ruff (0.14.10)
181-183: Avoid specifying long messages outside the exception class
(TRY003)
…ing capabilities. Added methods for loading and managing a template registry, including metadata retrieval and template validation. Updated initialization to support registry paths and refined rendering logic for various template types.
k-dot-greyz
force-pushed
the
feat-template-registry-engine-fad9c
branch
from
6eaa139 to
8ee5199
Compare
|
@coderabbitai full review Review mode: please run this like assertive (max feedback: potential issues, refactors, security/edge cases, and nitpicks where useful). If assertive is only controllable via OSS / free tier — use everything that applies (per CodeRabbit docs): full walkthrough with changed-file summary, effort estimate, sequence diagrams when they add value, related issues + related PRs, linked-issue fit assessment, suggested labels + reviewer suggestions, review status detail, collapsible walkthrough, and the “prompt for AI agents” style inline hints where supported. Finishing touches (if enabled for this repo/plan): call out missing docstrings / unit-test gaps; if generation commands are allowed, say so explicitly in review comments (I know autofix is Pro-only). Thanks — going for maximum signal without asking for paid-only automation. |
|
Assertive mode override applied — full feedback including security, edge cases, refactors, docstring/test gaps, and diagrams where they add value. ✏️ Learnings added
✅ Actions performedFull review triggered. |
1 participant
…ing capabilities. Added methods for loading and managing a template registry, including metadata retrieval and template validation. Updated initialization to support registry paths and refined rendering logic for various template types.
Pull Request Description
Summary
Briefly describe what this PR does.
Related Issues
Closes #
Type of Change
Pre-Commit Review Checklist
Remember to carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Code Review
Testing
Documentation
Other Considerations
Screenshots/Videos (if applicable)
Additional Notes
Note
Introduces a registry-driven template system with discovery, validation, and evolution metadata, plus a PR status checking workflow and tooling.
TemplateEnginewith registry loading (templates/registry.yaml), entry lookup,render_by_id, source reading, basic variable checks, and support for multiple template typeszen/templates/pokedex.py) for cataloging, rarity/evolution insights, and usage summaries; exposes viazen/templates/__init__.pyzen/templates/validator.py) for schema-based validation (JSON Schema), placeholder linting, and render checksdev-master/zen/cli.pyfor managing templates, registry, schemas, and evolution datatemplates/registry.yaml,templates/metadata/evolution.yaml, and JSON schemas undertemplates/metadata/schemas/**.github/workflows/pr-status-check.yml, local scriptscripts/check_prs.py, and docsscripts/README_PR_CHECK.mdWritten by Cursor Bugbot for commit 6eaa139. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.