Generate valid prompt frontmatter and canonical filenames#30
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdded shared utilities for canonical prompt Markdown filenames, YAML frontmatter, escape decoding, and collision checks. Updated prompt synchronization to preserve metadata and token counts across migrations, and added Skills categorization to README generation. ChangesPrompt Markdown synchronization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PromptJSON
participant updatePrompts
participant MarkdownFiles
participant Anthropic
PromptJSON->>updatePrompts: provide prompts
updatePrompts->>MarkdownFiles: resolve existing files by path, name, or body
updatePrompts->>Anthropic: count changed prompt bodies
Anthropic-->>updatePrompts: return token counts
updatePrompts->>MarkdownFiles: write Markdown and rebuild README
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Serialize every frontmatter string as deterministic YAML-safe output and derive generated paths with the documented non-alphanumeric slug rules. Add collision and serialization regression coverage, preserve README token counts across metadata-only and filename-only rewrites, and regenerate the complete 2.1.215 prompt corpus.
Fail before generation when a metadata scalar contains the HTML comment terminator, which YAML quoting cannot make safe inside the surrounding comment block. Cover the guard in the serialization regression test.
Regenerate the seven source-confirmed catalog corrections, replace all eleven zero token counts with real count-endpoint results, and make token counting fail closed instead of persisting zero on API errors. Preserve established counts across prompt renames by matching unchanged generated bodies.
d33bd1a to
7175f3e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tools/promptMarkdownUtils.mjs (1)
145-150: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
disallowedToolsscalar values are silently dropped, unliketools.
toolshas anelse if (metadata.tools != null)fallback to render a non-array scalar (Line 139-141), butdisallowedToolsonly handles theArray.isArraycase — a non-array truthydisallowedToolsvalue would be dropped from frontmatter entirely with no warning.Proposed fix for parity with `tools`
if (Array.isArray(metadata.disallowedTools)) { lines.push(' disallowedTools:'); for (const tool of metadata.disallowedTools) { lines.push(` - ${yamlString(tool)}`); } + } else if (metadata.disallowedTools != null) { + lines.push(` disallowedTools: ${yamlString(metadata.disallowedTools)}`); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/promptMarkdownUtils.mjs` around lines 145 - 150, Update the metadata serialization logic adjacent to the disallowedTools array handling to add the same non-array fallback used for tools. When disallowedTools is non-null and not an array, render it as a scalar instead of dropping it, while preserving the existing array serialization behavior.tools/updatePrompts.js (1)
301-349: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSolid multi-source matching with defensive body-collision handling.
The path→name→body fallback chain,
existingTokenCountslookup chain, and settingexistingFilesByBodytonullon duplicate bodies to avoid ambiguous matches are all well-reasoned for preserving token counts across renames/metadata-only edits. Confirmed the deferred cleanup (no inlineunlinkSyncon rename) is safe:promptsByFilenameis keyed only by canonical filenames, so a stale old-named file is correctly caught by the later "deleted files" pass (Line 371-372).One minor nit: when a rename is detected via the name/body fallback (new canonical
filepathdiffers from the old file), the console logsChanged:(Line 334) even though this is effectively a rename+possible-metadata-change, not an in-place edit — purely cosmetic, not worth a code change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/updatePrompts.js` around lines 301 - 349, Review the existing-file handling around existingFilesByPromptName, existingFilesByBody, and the “Changed:” log: no implementation change is required. The review identifies only a cosmetic distinction for rename detection via fallback matching, so preserve the current behavior unless you choose to adjust the message separately.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tools/promptMarkdownUtils.mjs`:
- Around line 145-150: Update the metadata serialization logic adjacent to the
disallowedTools array handling to add the same non-array fallback used for
tools. When disallowedTools is non-null and not an array, render it as a scalar
instead of dropping it, while preserving the existing array serialization
behavior.
In `@tools/updatePrompts.js`:
- Around line 301-349: Review the existing-file handling around
existingFilesByPromptName, existingFilesByBody, and the “Changed:” log: no
implementation change is required. The review identifies only a cosmetic
distinction for rename detection via fallback matching, so preserve the current
behavior unless you choose to adjust the message separately.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8cd352e8-e43c-45cf-bec1-24728e3c7509
📒 Files selected for processing (3)
tools/promptMarkdownUtils.mjstools/promptMarkdownUtils.test.mjstools/updatePrompts.js
2 participants
Why
The prompt release pipeline needs to reproduce the generated corpus deterministically. Before this change, the tracked generator could emit invalid YAML in HTML-comment frontmatter, derive filenames differently from the documented canonical algorithm, persist zero token counts after API failures, and rewrite source-escaped prompt punctuation literally during regeneration.
The 2.1.216 release already contains the corrected generated corpus. This PR codifies those rules in the tracked generator so the next release does not regress the metadata, paths, README structure, token counts, or prompt bodies.
What changed
tools/promptMarkdownUtils.mjswith deterministic YAML-safe string serialization, rejection of unsafe HTML comment terminators, the documented canonical filename algorithm, legacy/current generated-name parsing, source-escape decoding, and a pre-generation collision guard.tools/updatePrompts.jsto use the shared helpers for every frontmatter string and list item, including variables and all agent metadata fields.SkillandTool ParameterREADME categorization, corrected agent subsection heading levels, and normalized generated description punctuation.Literal example placeholders such as
${API_KEY}remain intentionally out of scope.Validation
node --test tools/promptMarkdownUtils.test.mjs— 6 passednode --check tools/updatePrompts.jsANTHROPIC_API_KEY=not-used-for-unchanged-bodies node tools/updatePrompts.js /code/tweakcc/data/prompts/prompts-2.1.216.json— 598 prompts; 0 new, 0 changed, 0 deletedpython3 /tmp/validate-pr30-output.py— 598 Markdown files, 598 README entries, 0 errors; 228 pre-existing interpolation-reference warnings remain unchanged frommaingit diff --check -- toolsSummary by CodeRabbit
New Features
Bug Fixes