Skip to content

Commit 2152985

Browse files
Merge pull request #83 from HendrikBorgelt/fix-inbox-script
Fix inbox script
2 parents 0780daa + e609ec9 commit 2152985

2 files changed

Lines changed: 94 additions & 8 deletions

File tree

.github/workflows/excel_inbox.yaml

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,15 @@ jobs:
122122
run: |
123123
# Run inbox_to_schema.py from _main_branch/ against the inbox Excel
124124
# (which lives in the PR branch working directory, one level up).
125+
#
126+
# set +e: GitHub Actions shells run with -eo pipefail, which means
127+
# a failed command substitution (OUTPUT=$(cmd)) would abort the shell
128+
# before we can write the output to GITHUB_ENV for the PR comment.
129+
set +e
125130
OUTPUT=$(uv run python scripts/inbox_to_schema.py \
126131
"../${{ env.INBOX_FILE }}" 2>&1)
127132
EXIT_CODE=$?
133+
set -e
128134
129135
echo "$OUTPUT"
130136
@@ -164,9 +170,12 @@ jobs:
164170
steps.apply.outcome != 'failure'
165171
id: test
166172
run: |
167-
# Capture output + exit code; shell continues even on failure (no set -e)
173+
# Capture output + exit code; disable set -e so a failing test suite
174+
# does not abort the shell before we can write to GITHUB_ENV.
175+
set +e
168176
TEST_OUTPUT=$(just test 2>&1)
169177
TEST_EXIT=$?
178+
set -e
170179
171180
echo "$TEST_OUTPUT"
172181
@@ -208,8 +217,10 @@ jobs:
208217
steps.regen_excel.outcome == 'success'
209218
id: roundtrip
210219
run: |
220+
set +e
211221
RT_OUTPUT=$(uv run python scripts/excel_to_schema.py \
212222
"../${{ env.INBOX_FILE }}" 2>&1)
223+
set -e
213224
214225
echo "$RT_OUTPUT"
215226

scripts/inbox_to_schema.py

Lines changed: 82 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -231,12 +231,30 @@ def build_label_index(schema: dict) -> tuple[dict[str, str], dict[str, str]]:
231231
"""
232232
Return (label_to_slot, label_to_class): inverse of snake_to_readable for
233233
every slot and class defined in the merged schema.
234+
235+
label_to_slot covers:
236+
• globally-defined slots (schema["slots"])
237+
• slots referenced only via class slot_usage blocks (e.g. had_input_entity,
238+
realized_plan) that are not in the global slots dict but appear in the
239+
Excel because _collect_rows uses get_class_ranged_slot_usage.
234240
"""
235241
label_to_slot: dict[str, str] = {}
236242
label_to_class: dict[str, str] = {}
237243

244+
# 1. Global slots
238245
for name in schema.get("slots", {}):
239246
label_to_slot[snake_to_readable(name)] = name
247+
248+
# 2. Slot-usage-only slots: referenced in class slot_usage but not globally
249+
# defined (e.g. had_input_entity in Synthesis.slot_usage). These are
250+
# rendered by schema_to_excel as top-level slot rows with an empty domain
251+
# and must be recognised as existing, not new.
252+
for class_def in schema.get("classes", {}).values():
253+
for slot_name in (class_def.get("slot_usage") or {}):
254+
label = snake_to_readable(slot_name)
255+
if label not in label_to_slot:
256+
label_to_slot[label] = slot_name
257+
240258
for name in schema.get("classes", {}):
241259
label_to_class[snake_to_readable(name)] = name
242260

@@ -477,23 +495,73 @@ def plan_changes(
477495
for row in rows:
478496
label = row["label"]
479497
row_type = row["type"]
498+
domain = row["domain"]
480499

481500
if row_type == "slot":
482501
slot_name = label_to_slot.get(label)
483-
if slot_name is None:
484-
_plan_new_slot(
485-
row, sheet_title, schema_class,
486-
schema, class_origin, slot_origin, label_to_slot,
487-
changes, reporter,
502+
503+
if slot_name is not None:
504+
# ── Known global slot ───────────────────────────────────
505+
# If the row has a domain (e.g. domain="Precursor"), the
506+
# slot lives in that subclass's slot_usage, not in the
507+
# top-level class. Use the domain class as context so
508+
# _plan_slot_changes targets the right YAML node.
509+
effective_class = (
510+
(label_to_class.get(domain) or domain)
511+
if domain else schema_class
488512
)
489-
else:
490513
seen_slot_names.add(slot_name)
491514
_plan_slot_changes(
492-
row, slot_name, schema, sheet_title, schema_class,
515+
row, slot_name, schema, sheet_title, effective_class,
493516
slot_origin, class_origin, changes, reporter,
494517
)
495518

519+
elif domain:
520+
# ── Unknown label + non-empty domain ───────────────────
521+
# Could be a slot_usage-only slot (not in global slots:)
522+
# defined on the domain class (e.g. has_concentration in
523+
# CoPrecipitation). Derive the slot name and check.
524+
effective_class = label_to_class.get(domain) or domain
525+
derived_name = _label_to_slot_name(label)
526+
cls_def = schema.get("classes", {}).get(effective_class, {})
527+
if derived_name in (cls_def.get("slot_usage") or {}):
528+
seen_slot_names.add(derived_name)
529+
_plan_slot_changes(
530+
row, derived_name, schema, sheet_title,
531+
effective_class, slot_origin, class_origin,
532+
changes, reporter,
533+
)
534+
else:
535+
# Slot belongs to the subclass hierarchy (mixin,
536+
# attribute, or imported slot) and cannot be modified
537+
# via the inbox workflow. Skip silently — these rows
538+
# are structural display information from the Excel
539+
# generator, not editable fields.
540+
reporter.info(
541+
sheet_title, f"slot '{label}'",
542+
f"Skipped: belongs to sub-class `{effective_class}` "
543+
f"and is not modifiable via the inbox workflow "
544+
f"(edit the YAML directly).",
545+
)
546+
547+
else:
548+
# ── Unknown label + empty domain → new top-level slot ──
549+
_plan_new_slot(
550+
row, sheet_title, schema_class,
551+
schema, class_origin, slot_origin, label_to_slot,
552+
changes, reporter,
553+
)
554+
496555
elif row_type == "class":
556+
# ── When a class row has a domain, that domain is the label
557+
# of the parent slot (e.g. "had input entity" for Precursor).
558+
# Mark that parent slot as seen so the deletion detector does
559+
# not falsely warn that it is missing from the workbook.
560+
if domain:
561+
parent_slot_name = label_to_slot.get(domain)
562+
if parent_slot_name:
563+
seen_slot_names.add(parent_slot_name)
564+
497565
class_name = label_to_class.get(label)
498566
if class_name is None:
499567
_plan_new_class(
@@ -1262,6 +1330,13 @@ def main(inbox_path: Path) -> int:
12621330

12631331

12641332
if __name__ == "__main__":
1333+
# Ensure stdout is UTF-8 even on Windows (emoji in Markdown output otherwise
1334+
# crash with UnicodeEncodeError on cp1252 consoles / cmd.exe).
1335+
if hasattr(sys.stdout, "reconfigure"):
1336+
sys.stdout.reconfigure(encoding="utf-8", errors="replace")
1337+
if hasattr(sys.stderr, "reconfigure"):
1338+
sys.stderr.reconfigure(encoding="utf-8", errors="replace")
1339+
12651340
inbox_path = Path(sys.argv[1]) if len(sys.argv) > 1 else DEFAULT_INBOX
12661341
if not inbox_path.exists():
12671342
print(

0 commit comments

Comments
 (0)