feat(contract): validate canonical frontmatter FIELDS, not just artifact shape - #93
Open
chitcommit wants to merge 1 commit into
Open
chitcommit wants to merge 1 commit into
chitcommit wants to merge 1 commit into
Conversation
…act shape
Patterning an artifact's SHAPE without typing its field VALUES only relocates
drift from section structure into field content. This encodes the currently
observed field contract so it can be attacked and corrected.
Scope boundary, stated in the file: this is a work engine, not an authority.
Data contracts are chartered to ChittySchema. Where the corpus and this file
disagree, the disagreement IS the finding — the file does not own the contract.
Against the real 59-artifact corpus it finds:
KIND_ALIAS_DRIFT mcp-server x2 — one sub-kind under two names
MISSING_REQUIRED classification absent on both canonical/tools/*
NO_FRONTMATTER canonical/README.md — UNPROVEN, not clean
Exit codes distinguish proven-clean (0), violations (1), and INCOMPLETE (2),
because a file that could not be parsed must never read as a file that passed.
canonical/README.md has no frontmatter, so this run exits 2 and says so.
Writing it immediately corrected two claims I had measured by hand and already
recorded as fact:
- "status: 3 of 4 values are malformed enum definitions" — FALSE. Those
strings are at lines 87/115/145, in the markdown BODY, not frontmatter;
two files are DOCUMENTING an enum. Frontmatter status is clean.
- "canon_uri: 9 undeclared namespaces + depth + trailing-slash drift" —
body-text noise from a greedy sed that ate through `chittycanon:`. Correct
field-value measurement is 3 namespaces over 59 declarers: core 49,
skills 9, commands 1.
Both errors over-stated drift in the same direction. The surviving canon_uri
finding is narrower and real: TWO competing addressing schemes in one corpus,
service-based core/services/* (49) against class-based skills/*+commands/* (10).
Method lesson encoded in the file's parser rather than a comment: a field-anchored
`^key:` grep cannot distinguish frontmatter from body. Parse the block.
mcp-server/mcp is reported as drift rather than silently normalised, because
which spelling is canonical is ChittySchema's call, not this file's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H6EB22mDgirBYomHn3hfyJ
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughAdds ChangesCanonical field contract validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant MarkdownTraversal
participant FrontmatterParser
participant ContractValidator
participant Reporter
CLI->>MarkdownTraversal: recursively scan canonical directory
MarkdownTraversal->>FrontmatterParser: parse Markdown frontmatter
FrontmatterParser->>ContractValidator: provide parsed fields or parse failure
ContractValidator->>Reporter: provide findings and scan status
Reporter-->>CLI: render human-readable or JSON output
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
scripts/canonical-field-contract.mjs (2)
113-121: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
walkfollows symlinks and can recurse without bound.
statSyncresolves symlinks. A symlinked directory that points to an ancestor makeswalkrecurse until the stack overflows, and the crash handler reports exit 3.readdirSyncwithwithFileTypesremoves the extrastatSyncsyscall per entry and does not follow symlinks.♻️ Proposed walker refactor
function walk(dir) { - for (const entry of readdirSync(dir)) { - const p = join(dir, entry) - const st = statSync(p) - if (st.isDirectory()) { walk(p); continue } - if (!entry.endsWith('.md')) continue - check(p) + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const p = join(dir, entry.name) + if (entry.isDirectory()) { walk(p); continue } + if (!entry.isFile() || !entry.name.endsWith('.md')) continue + check(p) } }Then drop
statSyncfrom the import on line 36.🤖 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 `@scripts/canonical-field-contract.mjs` around lines 113 - 121, Update walk to call readdirSync with withFileTypes enabled and use each Dirent’s isDirectory() result, so symlink entries are not traversed and the extra statSync call is removed. Preserve recursive traversal of real directories and .md file checking via check(p), and remove the now-unused statSync import.
198-209: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSet
process.exitCodeafter writing the report.
process.exitcan truncate pipedstdoutoutput before the JSON report is fully written. Setprocess.exitCodeand allow the process to terminate naturally.🤖 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 `@scripts/canonical-field-contract.mjs` around lines 198 - 209, Update the final exit handling after the report is written in the canonical field contract script: replace both process.exit calls with a single process.exitCode assignment that returns 2 for unproven results, 1 for findings, and 0 otherwise, allowing stdout to flush naturally.
🤖 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.
Inline comments:
In `@scripts/canonical-field-contract.mjs`:
- Around line 42-45: Update the ROOT validation in canonical-field-contract.mjs
to require an existing directory, using a directory-specific filesystem check
rather than existsSync alone. Preserve the existing usage message and exit 64
behavior for both missing paths and file paths, preventing readdirSync from
receiving a non-directory path.
- Around line 133-138: Update the file-sweep logic around the frontmatter check
to explicitly exclude the root documentation file canonical/README.md from
artifact validation before it can be added to unproven. Keep the exclusion
visible and narrowly scoped, while continuing to validate all other Markdown
files normally.
- Around line 92-111: Update parseFields to parse documented YAML flow sequences
such as runtimes: [] and classification: [...] into arrays, including non-empty
values, and strip surrounding quotes from scalar values before validation.
Ensure unsupported value shapes are represented as UNPROVEN rather than silently
skipped, while preserving existing block-list parsing behavior.
---
Nitpick comments:
In `@scripts/canonical-field-contract.mjs`:
- Around line 113-121: Update walk to call readdirSync with withFileTypes
enabled and use each Dirent’s isDirectory() result, so symlink entries are not
traversed and the extra statSync call is removed. Preserve recursive traversal
of real directories and .md file checking via check(p), and remove the
now-unused statSync import.
- Around line 198-209: Update the final exit handling after the report is
written in the canonical field contract script: replace both process.exit calls
with a single process.exitCode assignment that returns 2 for unproven results, 1
for findings, and 0 otherwise, allowing stdout to flush naturally.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bf119f74-b7b9-40c2-bd8c-a2813b47fba5
📒 Files selected for processing (1)
scripts/canonical-field-contract.mjs
Comment on lines
+42
to
+45
| if (!existsSync(ROOT)) { | ||
| console.error(`usage: canonical-field-contract.mjs [canonical-dir] [--json]\n no such directory: ${ROOT}`) | ||
| process.exit(64) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate that the argument is a directory, not only that the path exists.
If a caller passes a file path, existsSync passes and readdirSync throws ENOTDIR. The runner then reports exit 3 (runner crash) instead of exit 64 (usage error). Callers that distinguish exit codes get the wrong signal.
🐛 Proposed usage check
-if (!existsSync(ROOT)) {
+if (!existsSync(ROOT) || !statSync(ROOT).isDirectory()) {
console.error(`usage: canonical-field-contract.mjs [canonical-dir] [--json]\n no such directory: ${ROOT}`)
process.exit(64)
}📝 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.
Suggested change
| if (!existsSync(ROOT)) { | |
| console.error(`usage: canonical-field-contract.mjs [canonical-dir] [--json]\n no such directory: ${ROOT}`) | |
| process.exit(64) | |
| } | |
| if (!existsSync(ROOT) || !statSync(ROOT).isDirectory()) { | |
| console.error(`usage: canonical-field-contract.mjs [canonical-dir] [--json]\n no such directory: ${ROOT}`) | |
| process.exit(64) | |
| } |
🤖 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 `@scripts/canonical-field-contract.mjs` around lines 42 - 45, Update the ROOT
validation in canonical-field-contract.mjs to require an existing directory,
using a directory-specific filesystem check rather than existsSync alone.
Preserve the existing usage message and exit 64 behavior for both missing paths
and file paths, preventing readdirSync from receiving a non-directory path.
Comment on lines
+92
to
+111
| function parseFields(block) { | ||
| const out = {} | ||
| let key = null | ||
| for (const raw of block.split('\n')) { | ||
| if (!raw.trim() || raw.trimStart().startsWith('#')) continue | ||
| const top = raw.match(/^([a-z0-9_]+):\s*(.*)$/i) | ||
| if (top) { | ||
| key = top[1] | ||
| const v = top[2].trim() | ||
| out[key] = v === '' || v === '|' || v === '>' ? [] : v | ||
| continue | ||
| } | ||
| const item = raw.match(/^\s+-\s+(.*)$/) | ||
| if (item && key) { | ||
| if (!Array.isArray(out[key])) out[key] = [] | ||
| out[key].push(item[1].trim()) | ||
| } | ||
| } | ||
| return out | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect frontmatter value styles for the validated fields.
set -euo pipefail
fd -e md . canonical | while IFS= read -r f; do
awk 'NR==1 && $0!="---" {exit} NR>1 && $0=="---" {exit} {print FILENAME": "$0}' "$f"
done | rg -n '(runtimes|classification|kind|status|canon_uri|canonical_uri):' | head -80Repository: chittyos/chittymarket
Length of output: 5706
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- target script ---'
sed -n '80,185p' scripts/canonical-field-contract.mjs
printf '%s\n' '--- representative frontmatter ---'
python3 - <<'PY'
from pathlib import Path
fields = {"runtimes", "classification", "kind", "status"}
shown = 0
for path in Path("canonical").rglob("*.md"):
text = path.read_text()
if not text.startswith("---\n"):
continue
end = text.find("\n---", 4)
if end < 0:
continue
lines = text[4:end].splitlines()
selected = [line for line in lines if line.split(":", 1)[0].strip() in fields]
if selected:
print(path)
print("\n".join(selected))
shown += 1
if shown >= 12:
break
PY
printf '%s\n' '--- documented parser assumptions ---'
rg -n -C 3 'runtimes:|classification:|UNPROVEN|parseFields|Array\.isArray' \
docs scripts/canonical-field-contract.mjs | head -160Repository: chittyos/chittymarket
Length of output: 11236
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
from collections import Counter, defaultdict
def frontmatter(text):
if not text.startswith("---"):
return None
end = text.find("\n---", 3)
return None if end == -1 else text[4:end]
def parse_fields(block):
out = {}
key = None
for raw in block.split("\n"):
if not raw.strip() or raw.lstrip().startswith("#"):
continue
top = __import__("re").match(r"^([a-z0-9_]+):\s*(.*)$", raw, __import__("re").I)
if top:
key = top.group(1)
v = top.group(2).strip()
out[key] = [] if v in ("", "|", ">") else v
continue
item = __import__("re").match(r"^\s+-\s+(.*)$", raw)
if item and key:
if not isinstance(out[key], list):
out[key] = []
out[key].append(item.group(1).strip())
return out
raw_shapes = Counter()
parsed_shapes = Counter()
examples = defaultdict(list)
quoted = []
for path in Path("canonical").rglob("*.md"):
block = frontmatter(path.read_text())
if block is None:
continue
for field in ("runtimes", "classification", "kind", "status"):
lines = block.splitlines()
for i, line in enumerate(lines):
if line.lower().startswith(field + ":"):
value = line.split(":", 1)[1].strip()
if value in ("", "|", ">"):
shape = "empty/block-style"
elif value.startswith("[") and value.endswith("]"):
shape = "inline-flow-sequence"
elif value[:1] in ("'", '"'):
shape = "quoted-scalar"
else:
shape = "plain-scalar"
raw_shapes[field, shape] += 1
examples[field, shape].append((str(path), line))
break
parsed = parse_fields(block)
for field in ("runtimes", "classification", "kind", "status"):
if field in parsed:
parsed_shapes[field, "array" if isinstance(parsed[field], list) else "string"] += 1
print("RAW SHAPES")
for k, n in sorted(raw_shapes.items()):
print(f"{k[0]:15} {k[1]:24} {n}")
print("\nPARSED SHAPES")
for k, n in sorted(parsed_shapes.items()):
print(f"{k[0]:15} {k[1]:10} {n}")
print("\nEXAMPLES")
for (field, shape), rows in sorted(examples.items()):
print(f"{field} / {shape}:")
for row in rows[:3]:
print(" ", row[0], "=>", row[1])
PY
printf '%s\n' '--- reporting tail ---'
sed -n '185,212p' scripts/canonical-field-contract.mjsRepository: chittyos/chittymarket
Length of output: 2430
Make parseFields handle documented YAML values
The parser treats the three current runtimes: [] fields as strings, so line 171 skips runtime validation. Any non-empty runtimes: [...] value would be skipped in the same way. The parser also cannot represent documented classification: [...] values. If quoted scalars are valid, quotes cause false KIND_UNKNOWN and STATUS_UNKNOWN findings. Parse flow sequences and surrounding quotes, or report unsupported shapes as UNPROVEN.
🤖 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 `@scripts/canonical-field-contract.mjs` around lines 92 - 111, Update
parseFields to parse documented YAML flow sequences such as runtimes: [] and
classification: [...] into arrays, including non-empty values, and strip
surrounding quotes from scalar values before validation. Ensure unsupported
value shapes are represented as UNPROVEN rather than silently skipped, while
preserving existing block-list parsing behavior.
Comment on lines
+133
to
+138
| const block = frontmatter(text) | ||
| if (block === null) { | ||
| // No frontmatter at all: cannot be checked, and MUST NOT count as clean. | ||
| unproven.push({ file: rel, code: 'NO_FRONTMATTER', msg: 'no frontmatter block — contract unprovable' }) | ||
| return | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
canonical/README.md makes the sweep permanently exit 2, which blocks any trigger wiring.
Every .md file under the root is treated as an artifact. canonical/README.md is documentation, not an artifact, so it always lands in unproven and forces exit 2. The PR states the checker is not yet wired to a trigger; with this behavior it can never reach exit 0. scripts/lint-plugins.sh (lines 259-280) already skips README.md for the same reason.
Add an explicit non-artifact exclusion so exclusions stay visible rather than implicit.
🐛 Proposed exclusion
+// Documentation files under canonical/ are not artifacts. Excluded explicitly
+// so the exclusion is auditable rather than implicit.
+const NON_ARTIFACTS = new Set(['README.md'])
+
function walk(dir) {
for (const entry of readdirSync(dir)) {
const p = join(dir, entry)
const st = statSync(p)
if (st.isDirectory()) { walk(p); continue }
if (!entry.endsWith('.md')) continue
+ if (NON_ARTIFACTS.has(entry)) continue
check(p)
}
}🤖 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 `@scripts/canonical-field-contract.mjs` around lines 133 - 138, Update the
file-sweep logic around the frontmatter check to explicitly exclude the root
documentation file canonical/README.md from artifact validation before it can be
added to unproven. Keep the exclusion visible and narrowly scoped, while
continuing to validate all other Markdown files normally.
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
1 participant
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.
Patterning an artifact's SHAPE without typing its field VALUES only relocates drift from section structure into field content. This encodes the currently observed field contract so it can be attacked and corrected.
Scope boundary
Stated in the file itself: this is a work engine, not an authority. Data contracts are chartered to ChittySchema. Where the corpus and this file disagree, the disagreement is the finding — the file does not own the contract.
What it finds against the real 59-artifact corpus
KIND_ALIAS_DRIFTmcp-server×2 — one sub-kind under two namesMISSING_REQUIREDclassificationabsent on bothcanonical/tools/*NO_FRONTMATTERcanonical/README.md— UNPROVEN, not cleanExit codes distinguish proven-clean (0), violations (1), and INCOMPLETE (2), because a file that could not be parsed must never read as a file that passed. This run exits 2 and says so.
Writing it corrected two claims I had already recorded as fact
statusis clean.sedthat ate throughchittycanon:. Correct field-value measurement: 3 namespaces over 59 declarers —core49,skills9,commands1.Both errors over-stated drift in the same direction. The surviving
canon_urifinding is narrower and real: two competing addressing schemes in one corpus — service-basedcore/services/*(49) against class-basedskills/*+commands/*(10).Method lesson is encoded in the parser rather than a comment: a field-anchored
^key:grep cannot distinguish frontmatter from body. Parse the block.Deliberately not normalised
mcp-server/mcpis reported as drift rather than silently fixed — which spelling is canonical is ChittySchema's call, not this file's.Not wired to anything yet
Like
instance-trollandai-search-schema-loop, this is averify-class capability with no trigger. That gap is real and tracked separately; this PR lands the checker, not its injection point.🤖 Generated with Claude Code
https://claude.ai/code/session_01H6EB22mDgirBYomHn3hfyJ
Summary by CodeRabbit
canonical/directory or a specified location.