fix(ci): address PR review — filter comments, trim whitespace, warn on missing scripts
Address all review comments from Hoshe and Tyre on PR #27: - Remove 2>/dev/null from pre-pr-fixtures (critical: swallowed errors) - Remove dead _file_type function - Check 4: error on districts with no locations declared - Check 5: print advisory message when skipping - Check 8: cross-file line ID uniqueness (not just per-file) - Check 9: document D-034 asymmetric relationships in docstring - Document regex fallback rationale in _scan_knowledge - Add D-035 decision trace to schema descriptions - Use concrete protocol version in DEVOPS.md example - Amend D-035 with focused (9th mood) and greeting (14th situation) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
+30
-20
@@ -82,17 +82,6 @@ def _load_yaml(path: Path) -> dict | list | None:
|
||||
return None
|
||||
|
||||
|
||||
def _file_type(path: Path) -> str | None:
|
||||
"""Determine content type from path (npc, dialogue, monologue, district, triangle)."""
|
||||
rel = path.relative_to(CONTENT_DIR)
|
||||
for part in reversed(rel.parts[:-1]):
|
||||
if part in ("npcs", "dialogue", "monologue", "triangles", "locations"):
|
||||
return part
|
||||
if path.name == "district.yaml":
|
||||
return "district"
|
||||
return None
|
||||
|
||||
|
||||
def _district_dir(path: Path) -> Path | None:
|
||||
"""Find the district directory containing this file."""
|
||||
parts = path.resolve().parts
|
||||
@@ -162,7 +151,8 @@ class ContentIndex:
|
||||
continue
|
||||
data = _load_yaml(path)
|
||||
if not data or not isinstance(data, dict):
|
||||
# Try line-by-line grep for fact_id definitions
|
||||
# Stub catalogs (comment-only or flat key-value) don't parse
|
||||
# as dicts. Fall back to regex extraction of fact_id lines.
|
||||
try:
|
||||
with open(path) as f:
|
||||
for line in f:
|
||||
@@ -289,7 +279,12 @@ class ContentIndex:
|
||||
continue
|
||||
district_key = str(district_dir)
|
||||
district_locs = self.locations.get(district_key, set())
|
||||
if district_locs and location not in district_locs:
|
||||
if not district_locs:
|
||||
print(f'XREF ERROR: dialogue location "{location}" references district with no locations declared')
|
||||
print(f" In: {_rel(path)}")
|
||||
print(f" District: {_rel(district_dir / 'district.yaml')}")
|
||||
errors += 1
|
||||
elif location not in district_locs:
|
||||
print(f'XREF ERROR: dialogue location "{location}" not in district')
|
||||
print(f" In: {_rel(path)}")
|
||||
print(f" District locations: {', '.join(sorted(district_locs))}")
|
||||
@@ -299,7 +294,7 @@ class ContentIndex:
|
||||
def _check_5_fact_ids(self) -> int:
|
||||
"""Check that all referenced fact_ids resolve to knowledge catalogs."""
|
||||
if not self.fact_ids:
|
||||
# Advisory mode: catalogs not populated yet
|
||||
print(" Check 5 (fact_ids): SKIPPED — no canonical fact_ids in knowledge catalogs yet")
|
||||
return 0
|
||||
|
||||
errors = 0
|
||||
@@ -414,31 +409,46 @@ class ContentIndex:
|
||||
return warnings
|
||||
|
||||
def _check_8_dialogue_line_ids(self) -> int:
|
||||
"""Check that dialogue line IDs are unique within each file."""
|
||||
"""Check that dialogue line IDs are unique across all dialogue files."""
|
||||
errors = 0
|
||||
# Cross-file uniqueness: same line ID in two files is an error
|
||||
global_seen: dict[str, Path] = {}
|
||||
for path, data in self.dialogue_files:
|
||||
lines = data.get("lines", [])
|
||||
if not isinstance(lines, list):
|
||||
continue
|
||||
seen: dict[str, int] = {}
|
||||
local_seen: dict[str, int] = {}
|
||||
for i, line in enumerate(lines, 1):
|
||||
if not isinstance(line, dict):
|
||||
continue
|
||||
line_id = line.get("id")
|
||||
if not line_id:
|
||||
continue
|
||||
if line_id in seen:
|
||||
# Per-file duplicate
|
||||
if line_id in local_seen:
|
||||
print(f'XREF ERROR: duplicate dialogue line id "{line_id}"')
|
||||
print(f" In: {_rel(path)}")
|
||||
print(f" First: line {seen[line_id]}")
|
||||
print(f" First: line {local_seen[line_id]}")
|
||||
print(f" Duplicate: line {i}")
|
||||
errors += 1
|
||||
else:
|
||||
seen[line_id] = i
|
||||
local_seen[line_id] = i
|
||||
# Cross-file duplicate
|
||||
if line_id in global_seen and global_seen[line_id] != path:
|
||||
print(f'XREF ERROR: dialogue line id "{line_id}" used in multiple files')
|
||||
print(f" First: {_rel(global_seen[line_id])}")
|
||||
print(f" Also in: {_rel(path)}")
|
||||
errors += 1
|
||||
elif line_id not in global_seen:
|
||||
global_seen[line_id] = path
|
||||
return errors
|
||||
|
||||
def _check_9_bidirectional_relationships(self) -> int:
|
||||
"""Check that NPC relationships have reciprocal entries."""
|
||||
"""Check that NPC relationships have reciprocal entries.
|
||||
|
||||
WARNING severity: asymmetric relationships are valid by design (D-034
|
||||
allows one-directional awareness). This check flags them for review,
|
||||
not as errors."""
|
||||
warnings = 0
|
||||
# Build map: canonical_id -> set of relationship targets
|
||||
npc_rels: dict[str, set[str]] = {}
|
||||
|
||||
Reference in New Issue
Block a user