fix(workshop,workflow-creator): close verified code-review gaps (v5.68.4)#53
Merged
Conversation
…8.4)
Fixes 16 confirmed correctness findings across the workshop + workflow-creator
ultracode machinery, plus 6 low-risk cleanups:
1. wc-audit.js: selective re-audit re-verifies freshly-reviewed clusters (was
skipping ALL verification whenever onlyChecks was set — always true per
SKILL.md's Mode-3 loop); verifier corrections now write back into the
returned review records so refuted phantoms don't re-flag forever.
2. workshop-verify.js: the Constraint leg now parses check-all.py's actual
JSON output (failed[]/errors[]) instead of hunting for "FAIL:" lines that
are never emitted — constraintFailures block, constraintErrors are a
non-blocking infra-only minor.
3. skills/workshop/SKILL.md: fixed documented return-shape (sections/
sectionsThatFailed, not slides/slidesThatFailed) so the retry path
actually scopes re-generation instead of regenerating every section.
4. wc_file_set.py: strip inline `# ...` comments before validating manifest
fields — the module's own docstring example was failing its own parser.
5. workshop-verify.js: selective re-review now re-verifies diagrams tied to
re-reviewed slides (was vacuously passing Visual when ONLY was set).
6. workshop-generate.js: an independent low-effort probe agent re-greps each
drafted section's fragment for cited inventory ids; the gate trusts the
probe's tokens over the section agent's self-report (gateProbe doctrine).
7. overflow-check.py: triggers on `tinymist compile` (not just `typst
compile`) and extracts the LAST *.typ token in the command segment
(tolerates leading flags like --input/--root).
8. find-slide-page-inject.py: TRIGGER_RE now matches any *.typ compile target,
not just paths under a literal `slides/` directory.
9. workshop-verify.js: null-guards on the Discover/Mechanical single-agent
results instead of crashing on TypeError.
10. GATE_BLOCKED_TOOLS landmine closed at 3 sites: workflow-creator's gate
exemplar, phase-gate-guard.py's docstring, and workshop's Phase 2->3
OUTLINE_APPROVED gate (workshop-phase-gate-guard.py now also gates the
Workflow(name="workshop-generate") dispatch, not just Edit/Write).
11-16. Deleted dead title_slug/titleSlug fields (workshop_slide_table.py,
kept section_slug() — it has a live test consumer); fixed the SOURCES.md
double-read; deleted an empty if-block; hoisted SEV_RANK consts;
wc-constraint-check.py now imports parse_frontmatter/skill_matches from
scripts/load-constraints.py instead of a regex that missed the scalar
`applies-to: x` form; collapsed per-page pdftotext calls to one -layout
pass split on form-feeds; fixed a stale "P22-26" label to "P22-30".
Deferred (not attempted, need a product decision or larger redesign): 3N->N
per-slide reviewer consolidation; per-gap->per-cluster wc verify batching;
P01-P30 rubric pre-extraction; deleting the LLM Discover fallback in
workshop-generate; wc-generate content round-trip slimming; P22-P30
inline-vs-grep dual source restructure; workshop-revise SOURCES_VERIFIED
lockout for pre-gate decks.
Testing: node --check on every edited workflows/*.js; py_compile on every
edited hook/script; full existing suites green (tests/wc_file_set_test.py,
tests/workshop_slide_table_test.py, tests/workshop_guard_test.py,
tests/workshop-engine-discover.test.mjs, tests/check_all_applies_to_test.py,
tests/dev-run-driver.test.mjs, tests/ds-run-driver.test.mjs,
tests/ds-grain-pause.test.mjs, tests/writing-engine-discover.test.mjs); new
tests/overflow_check_test.py (regex trigger/target variants + fail-open);
new wc_file_set docstring-example regression case; REPL-verified
find-slide-page-inject TRIGGER_RE against both `tinymist compile
presentation/slides.typ` and `tinymist compile slides/lecture1.typ`;
workshop_slide_table.py smoke-tested against a synthetic OUTLINE/SOURCES
fixture. Updated tests/workshop-engine-discover.test.mjs's 2 assertions that
encoded the pre-fix behavior (probe-agent label count, constraint-caveat
text).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J3NEAUUosUHyci4iFZBH2J
Merged
5 tasks
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
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.
Summary
Fixes 16 confirmed code-review findings in the workshop + workflow-creator ultracode machinery, plus 6 low-risk cleanups. Ordering note: this targets v5.68.4, assuming pending PRs #50 (5.68.1), #51 (5.68.2), #52 (5.68.3) land first in that order — rebase/re-bump if the actual merge order differs.
This PR intentionally does not touch any file owned by those pending PRs (
hooks/mechanical-floor-gate.py,hooks/writing-mechanical-gate.py,hooks/_gate_common.py,skills/docx-repair/**,workflows/writing-draft.js,skills/de-ai-revise/**,scripts/lib/plan_table_core.py,scripts/dev/**,scripts/ds/**,workflows/templates/**) or.planning/.What / why, per finding
if (!dim || ONLY) continuedisabled ALL adversarial verification on every Mode-3 re-audit (onlyChecksis always set per SKILL.md's loop), letting unverified phantom criticals block/goalconvergence forever. Fixed toif (!dim || (ONLY && !ONLY.has(c.key))) continueso freshly-reviewed clusters get verified. Also: verifier corrections were applied only toscoreByIdand never written back into the returnedreviewsrecords, sopriorReviewson the next re-audit carried the pre-correction score andreviewersThatFlaggedre-flagged refuted phantoms forever — corrections now mutate the review records in place.check-all.pynever emits (it prints JSON{passed,failed,conventions,errors,skipped}+ a summary), soconstraintFailureswas systematically empty and the gate never blocked on real constraint violations. Rewrote the prompt + schema to parse the JSON:failed[]→constraintFailures(blocking),errors[]→ newconstraintErrorsfield (one non-blocking minor finding, since exit-code 1 also fires on infra errors alone — conflating infra failure with deck defects).constraintsPassedis nowfailed[] empty, not the exit code.workshop-generatereturningslides/slidesThatFailed; the JS actually returnssections/sectionsThatFailed. The documented retry passedonlyChecks=undefined, regenerating every section on any failure. Fixed the two lines to the real keys._field()didn't strip inline# ...comments, so the module's own docstring example (midpoint: fix # one of: ...) failed validation against_MIDPOINTS. Added_strip_inline_comment(), applied in_field()and the constraint-line parser. Added a regression test asserting the docstring's own example now parses clean.!ONLY, anddiagramReviewswere never carried viapriorReviews, so a selective re-verify (workshop-revise re-checking edited slides) skipped visual review entirely and the gate vacuously passed ([].every() === true, 0 defects). Now, whenONLYis set, diagrams belonging to a slide inONLYare still reviewed; the scoreTable's Visual row says "skipped (carry-forward)" instead of a false ✅ when no diagrams on the reviewed slides exist.fidelityOktrusted the section agent's self-reportedcitedInventory(it greps its own fragment and echoes the result — nothing deterministic re-checked it). Added one cheap, low-effort probe agent per drafted section, pipelined onto the draft (no extra barrier), that independently re-runs the exact grep and returns the token list; the gate now trusts the probe's tokens over the self-report (mirrors run-core's independentgateProbedoctrine)."typst compile"missedtinymist compile(what workshop-generate.js and visual-verify actually invoke); (b) the regex required the.typtarget to be the first token aftercompile, sotypst compile --input handout=true slides.typnever matched. Fixed to trigger on both compilers and extract the last*.typtoken in the ;/&/|-delimited segment. Addedtests/overflow_check_test.pycovering both fixes + fail-open behavior.TRIGGER_RErequired a literalslides/directory, sopresentation/slides.typor flatslides.typdecks never triggered the heading→page injection. Broadened to any\S+\.typtarget; REPL-verified against both forms.disc.slides.length,!mech.slidesCompiled) would throwTypeErrorif either single-agent call returned null. Added a hard error for!discand a synthesized-critical-finding early return for!mech(mirrors the existing compile-failure early exit).skills/workflow-creator/SKILL.md) now includesGATE_BLOCKED_TOOLS=Agent+ a landmine sentence; (b)hooks/phase-gate-guard.py's docstring example fixed the same way; (c) workshop's Phase 2→3OUTLINE_APPROVEDgate —workshop-phase-gate-guard.py(a bespoke script, not built on the generic env-var pattern) now also gatesWorkflow(name="workshop-generate", ...)dispatch, not just directEdit/Writetoslides.typ/notes.typ; the SKILL.md matcher now includesWorkflow.Low-risk cleanups
workshop_slide_table.py— deleted deadtitle_slug/titleSlug(zero downstream consumers; verified with grep) and the field's two call sites; keptsection_slug()itself sincetests/workshop_slide_table_test.pyexercises it directly. Fixed the SOURCES.md double-read (read once, reused for both the paper-path extraction and the inventory-id scan).workshop-verify.js— deleted an emptyif (ungrounded > 0) {}block.SEV_RANKconst in bothworkshop-generate.jsandworkshop-verify.js(matching the wc-* files' pattern).hooks/wc-constraint-check.py— replaced the hand-rolledapplies-toregex (which only matched the bracketed-list form) withparse_frontmatter/skill_matchesimported by path fromscripts/load-constraints.py; the old regex silently defaulted a scalarapplies-to: workflow-creatorto "no match → all", leaking wc-only constraints onto every repo edit.workflows/wc-audit.js— fixed a stale "(P22-26 scored)" label to "(P22-30 scored)".workshop-verify.jsMechanical prompt — changed the per-pagepdftotext -f N -l Ninstruction to onepdftotext -layout file -pass split on form-feeds.Deferred (not attempted — listed per instructions)
3N→N per-slide reviewer consolidation; per-gap→per-cluster wc verify batching; P01-P30 rubric pre-extraction; deleting the LLM Discover fallback in workshop-generate; wc-generate content round-trip slimming; P22-P30 inline-vs-grep dual source restructure; workshop-revise SOURCES_VERIFIED lockout for pre-gate decks (needs a product decision on the escape hatch).
Test plan
node --checkon every editedworkflows/*.js(wc-audit.js, workshop-verify.js, workshop-generate.js)${}re-read of every edited prompt string (no template-literal breakage — known incident class)py_compileon every edited.py(wc_file_set.py, overflow-check.py, find-slide-page-inject.py, phase-gate-guard.py, workshop-phase-gate-guard.py, wc-constraint-check.py, workshop_slide_table.py)tests/wc_file_set_test.py(20/20),tests/workshop_slide_table_test.py(37/37),tests/workshop_guard_test.py(6/6),tests/workshop-engine-discover.test.mjs(26/26, 2 assertions updated to match the intended fix),tests/check_all_applies_to_test.py(13/13),tests/dev-run-driver.test.mjs,tests/ds-run-driver.test.mjs,tests/ds-grain-pause.test.mjs,tests/writing-engine-discover.test.mjs,tests/test_mechanical_floor_gate.py,tests/test_writing_mechanical_gate.py,tests/writing_guards_test.py,tests/writing_section_index_test.py,tests/writing_gate_probe_test.py,tests/test_law_review_footnote_symbols.py,tests/test_writing_constraint_lints.pytests/overflow_check_test.py— trigger/target regex variants (bare,--inputfirst, tinymist,--root, cd-prefix, non-triggering) + fail-open on malformed/empty/non-Bash stdinfind-slide-page-inject.py'sTRIGGER_REagainst bothtinymist compile presentation/slides.typandtinymist compile slides/lecture1.typworkshop_slide_table.pyagainst a synthetic OUTLINE.md/SOURCES.md fixture (--jsonoutput, notitleSlug, no crash)workshop-phase-gate-guard.py's newWorkflowgate: deniesworkshop-generatedispatch withoutOUTLINE_APPROVED.md, allows oncestatus: APPROVED, ignores unrelated workflow names, fails open on malformed stdin.claude-plugin/plugin.json,.claude-plugin/marketplace.jsonmetadata + plugin entry)🤖 Generated with Claude Code
https://claude.ai/code/session_01J3NEAUUosUHyci4iFZBH2J