Skip to content

Commit 70fa5e2

Browse files
rafaelscostaclaude
andauthored
fix(ids): skip unfilled placeholders in extractPurpose priority chain (#749)
* fix(ids): scope purpose inference + sync self-entry on regen Closes the cosmetic follow-up noted after PR #747 — CodeRabbit's "Outside diff range" comments about entity-registry self-entry drift and the `component-creation-guide` purpose field reading "Analyzes provided dataset to identify patterns and insights" (clearly unrelated to a component creation guide). ## Bug 1: purpose inference matched body text it shouldn't have `extractPurpose()` in `populate-entity-registry.js` had a fallback regex: ```js const descMatch = content.match(/(?:description|purpose|summary)[:]\s*(.+)/i); ``` This is case-insensitive and unanchored — it matched ANY occurrence in the file body, including example output inside installer transcripts and fenced code blocks. For `component-creation-guide.md`, it picked up line 132: ``` ? Task description: Analyzes provided dataset to identify patterns and insights ``` …which is example output showing what a user might type, not a description of the guide itself. Fix: restructured `extractPurpose()` to a stricter priority chain: 1. **YAML frontmatter** — `description:` (or aliases) inside the `---` block at the top of the file. Anchored to the frontmatter region so body matches can't leak in. 2. **`## Purpose` section** — first non-empty line. 3. **`## Overview` section** — same shape as Purpose. Many guides use Overview instead (e.g. component-creation-guide.md itself). 4. **First `# Title` heading** — the document's name. 5. **Generic fallback** — `Entity at <path>`. The previous body-level `(?:description|purpose|summary)[:]` regex was REMOVED deliberately. Its false-positive rate (matching example output, installer transcripts, code block content) outweighed the narrow legitimate case it covered, which is now handled by the frontmatter branch. ## Bug 2: self-entry drift in `entities.data.entity-registry` The registry contains a record for itself because the data layer scan picks up every `.yaml`. That record's `lastVerified` was stuck at whatever value the previous scan happened to write, and the `checksum` was likewise stale relative to the regenerated file. Fix: added `syncSelfRegistryEntry()` which runs after the registry object is assembled but before yaml.dump. It: - Sets `lastVerified` to the same `lastUpdated` timestamp written into `metadata.lastUpdated` (single source of truth for this regen event). - Sets `checksum` to a sentinel value `sha256:<self-reference>` because hashing the registry from inside the registry is circular — any computed hash invalidates itself the moment it's written. The sentinel signals "skip this comparison" to downstream validators; trying to compute a "real" hash here would just produce a number that doesn't match its own input. The sync is guarded: it only fires when the self-entry's `path` field is `.aiox-core/data/entity-registry.yaml`. The companion `entity-registry.js` module under `entities.modules.*` is NOT self-referential and is left alone. ## Validation Regenerated the registry on this branch. After the fix: - `grep -c "Analyzes provided dataset" .aiox-core/data/entity-registry.yaml` → `0` - `grep -c "Add MCP Server Task" .aiox-core/data/entity-registry.yaml` → `0` (was the wrong purpose on the yaml self-entry) - The yaml self-entry's `lastVerified` now matches `metadata.lastUpdated` exactly - The yaml self-entry's `checksum` is the sentinel as expected The `component-creation-guide` entity is no longer in the registry at all — the file lives under `.aiox-core/core/docs/` which is not in `SCAN_CONFIG` (the `modules` scan covers `.aiox-core/core` but only for `*.js`/`*.mjs`, not `*.md`). Its previous presence was an artifact of a prior scan run with broader config; this regen cleans it up. If we want to start tracking framework docs as entities, that's a separate concern handled in a follow-up to SCAN_CONFIG. ## Out of scope for this commit A second-order cleanup that this PR does NOT do: there are still some purposes in the registry that look like leaked template placeholders (`'{Brief description of what this task does...}'`, `'{{TASK_TITLE}}'`, `'*${taskName.replace(...)}'`). Those come from files whose own bodies contain unfilled handlebars or JS literals — the inference is technically working, the source is the problem. Separate from this fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(ids): align extractPurpose tests with refactored priority chain The previous suite had a test "extracts from description field" that fed a bare `description: ...` string and expected it to be matched. That was exactly the body-level fallback regex that produced the "Analyzes provided dataset" false purpose on component-creation-guide.md (reported by CodeRabbit on PR #747). The refactor in this PR intentionally removed that match, so the test must be updated to reflect the new contract. Changes to the test file: * Replaced "extracts from description field" with two replacement tests: - "extracts from YAML frontmatter `description:` field" — feeds a `---\ndescription: X\n---` block and expects extraction from it. - "handles quoted YAML frontmatter description" — feeds `description: "X"` inside frontmatter and expects the quotes to be stripped. * Added "falls back to ## Overview when no Purpose section" — covers the new strategy 3 in the priority chain (many guides like component-creation-guide.md use Overview instead of Purpose). * Added TWO regression tests for the bug that motivated the refactor: - "does NOT match body-level `description:` outside frontmatter" — reproduces the exact component-creation-guide pattern (Overview section + a fenced code block with `Task description:` inside) and asserts the Overview wins. - "does NOT extract from `description:` lines without YAML frontmatter delimiters" — bare `description: X` without `---` delimiters falls through to the `# Title` header, NOT to the body line. Test result: 79/79 pass. Existing 5 tests in the suite still pass. Two new tests cover the YAML frontmatter branch (priority 1) and the new `## Overview` branch (priority 3). Two regression tests lock in the removal of the body-level matcher so it can't silently come back. * fix(ids): skip unfilled placeholders in extractPurpose priority chain Follow-up to PR #748. With body-level matching gone, the priority chain still picked up garbage purposes from files that have literal unfilled template placeholders or JS interpolation in the SOURCES the chain reads: YAML frontmatter, `## Purpose` sections, `# Title` headings. The extractor was technically working — the source data was just full of `{Brief description...}`, `{{TASK_TITLE}}`, and `${context.componentName}` strings that nobody ever filled in. ## What this PR does Adds `looksLikePlaceholder(candidate)` that detects: 1. Whole-string single placeholder: `{Brief...}`, `{{var}}`, `${ctx.x}` 2. Leading-token placeholder: `*${name}foo`, `${ctx.x} bar`, `{Title} extra` 3. Dominant interpolation (>30% of string): catches `${icon} @${id} — ${name}${archetype !== 'Specialist' ? ` (${archetype})` : ''} | ${title}` where most of the string is `${}` blocks The detector is conservative on real prose: - `Manage feature flags with { enabled: true } syntax` → keeps - `Use this skill when... ${variable not present}` (<30% interpolation) → keeps - Empty / whitespace / nullish → keeps (caller handles) Wired into all four `extractPurpose` strategies (frontmatter, `## Purpose`, `## Overview`, `# Title`). When a strategy produces a placeholder, it falls through to the next strategy. When all four produce placeholders, the function returns the generic `Entity at <path>` fallback rather than a nonsense purpose. ## Validation Counted placeholders in `.aiox-core/data/entity-registry.yaml` before and after this PR: ``` $ grep -E "purpose: ['\"]?\{[A-Z]|purpose:.*\{\{|purpose:.*\\\$\\{" \ .aiox-core/data/entity-registry.yaml | wc -l # Before: 16 # After: 0 ``` Examples of entries that now have sensible purposes (or sensible generic fallbacks) instead of unfilled templates: - `*${taskName.replace(/-/g, '-')}` → now resolves via header or falls to `Entity at <path>` - `{Brief description of what this task does and when to use it}` → now falls through to the next strategy - `Generated: ${new Date().toISOString()}` → captured by dominant- interpolation heuristic and skipped - `${icon} @${id} — ${name}${archetype !== ...}` → captured by dominant-interpolation heuristic and skipped ## Tests 12 new tests added to `tests/core/ids/populate-entity-registry.test.js`: - 4 cover `extractPurpose` falling through placeholders at each strategy boundary - 8 cover `looksLikePlaceholder` directly (positive cases for each pattern + negative cases for normal prose that should NOT trip it) Suite: 91/91 pass (was 79/79 on PR #748). ## Why not just fix the source files? The placeholders live in templates and generators — they're SUPPOSED to be unfilled when stored in the framework. They get filled at use- time when the template instantiates an output. The registry's inference is reading the template body as if it were a finished doc, which is the actual bug. Fixing the inference (this PR) is correct; trying to fill the templates would break their template-ness. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(ids): add 5 regression tests for syncSelfRegistryEntry Addresses CodeRabbit feedback on PR #749 — the self-entry sync logic introduced in PR #748 was load-bearing (lastVerified + checksum drift was the original CodeRabbit complaint) but lacked targeted tests. Tests added cover the four invariants: 1. **Self-path match → mutates both fields**: when the entry's path equals `.aiox-core/data/entity-registry.yaml`, `lastVerified` becomes the new timestamp and `checksum` becomes the sentinel. 2. **Non-self path → no mutation**: the companion entry pointing at `.aiox-core/core/doctor/checks/entity-registry.js` (a real JS module, NOT self-referential) must NOT be touched. Catches the most likely regression: if the path guard is removed, the JS module entry would get clobbered with the yaml sentinel. 3. **Missing entities.data** is a no-op (defensive). 4. **Missing self-entry** is a no-op + leaves other data entries alone (defensive). 5. **Malformed registry** (`{}`, `null`) does not throw. Required exporting `syncSelfRegistryEntry` from the script's `module.exports` and consuming it in the test file. Suite: 96/96 pass (was 91/91). --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1f97f6f commit 70fa5e2

4 files changed

Lines changed: 1058 additions & 843 deletions

File tree

0 commit comments

Comments
 (0)