Skip to content

fix(dev/ds compiled-runner): pipe-safe parsing, N/A-test convention sync, ds outputsProduced forcing function (v5.68.3)#52

Merged
edwinhu merged 2 commits into
mainfrom
worktree-agent-ac37dc2e18d205594
Jul 5, 2026
Merged

fix(dev/ds compiled-runner): pipe-safe parsing, N/A-test convention sync, ds outputsProduced forcing function (v5.68.3)#52
edwinhu merged 2 commits into
mainfrom
worktree-agent-ac37dc2e18d205594

Conversation

@edwinhu

@edwinhu edwinhu commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes 8 verified code-review findings on the pass-#9 shared compiled-runner core (scripts/lib/plan_table_core.py, workflows/templates/run-core.js, and the dev/ds fragments/compilers). Version bump assumes PR #50 (5.68.1) and the pending round-2 PR (5.68.2) land before this one — rebase/renumber if that ordering changes.

What / why, per finding

  1. Pipe-safe cell splitting (high reachability). plan_table_core.split_row/find_table split on bare |, so a Verify cell with a shell pipe (`pytest -q | tail`) or regex alternation (`grep -E 'foo|bar'`) shifted every later column — this was actively corrupting the repo's own .planning/PLAN.md. New _split_cells() helper is aware of escaped \| (unescaped to a literal pipe) and pipes inside backtick code spans; both split sites now share it.

  2. dev N/A-test convention drift. dev-task.js's testRequired() and the parser's now-deleted _TEST_NA disagreed on em/en-dash cells (/-): the parser treated them as N/A, the compiled gate's predicate did not — a dash Failing-Test cell compiled but could never satisfy its own gate. dev-task.js is now the sole owner of the predicate (cross-referencing comment in both files); the parser's dead test_required/_TEST_NA are removed.

  3. ds prefix-tolerant header check. ds's required-column check used exact c not in header while cell extraction is prefix-tolerant (dev already used the shared has_col()) — a qualified header like Expected Output (per requirement) was falsely rejected as missing. Now uses plan_table_core.has_col().

  4. Restored ds's outputsProduced forcing function. The run-core extraction silently weakened it from schema-required + === true to optional + !== false. Restored via a compiled config hole __REQUIRE_OUTPUTS_PRODUCED__: ds_compile.py sets True, dev_compile.py sets False; run-core.js gates on it in both TRANSFORM_SCHEMA.required and the pass/collect logic — no domain names leak into the shared core.

  5. Deps-cell phantom-dep guard. parse_deps swept the whole cell for digit-looking tokens, so free text like T1 (needs config v2) created a phantom dep on v2. Now splits on commas/whitespace (after stripping after /backticks) and requires each piece to fullmatch a dep token; non-conforming residue is dropped and, via an optional violations list threaded from both domain parsers, reported instead of silently becoming a dependency.

  6. Mixed-level skip stamping. run-core.js's mixed-level branch (some tasks done, some todo) never stamped a state entry for blind-skipped done tasks, so collect() under-reported tasksDone/over-reported tasksRemaining and scoreTable showed · for completed work. Now stamps the same skip-state shape the all-done branch uses.

  7. Cleanup. Dropped the dead cfg.clearedRecheck alias (no producer exists). recheckTrigger().atLevel was always an echo of the li argument — the core now supplies atLevel: li directly; dev-task.js no longer returns the field.

  8. Guard-hook cleanup. dev-plan-executable-guard.py's docstring wrongly claimed it's wired via dev-plan-reviewer frontmatter (that skill is read-only, never writes PLAN_REVIEWED.md) — corrected to dev-design/SKILL.md. Both guard hooks now import their parser lazily inside validate_plan(), so the hundreds of unrelated Write/Edit hook firings per session skip the parser import.

Deferred (not attempted)

  • Extracting the duplicated compile driver (dev_compile/ds_compile) and guard shells into shared modules
  • Dataflow (non-barrier) scheduling in run-core
  • The pre-probe skip optimization
  • ds_compile._tier keyword-sniffing → a real Tier column
  • Unifying the Python/JS toposort tie-breakers — added a code comment at run-core.js's toposort noting it must stay order-compatible with plan_table_core.toposort_ids

Test plan

  • uv run python3 tests/plan_table_core_test.py — 30 passed, 0 failed (10 new)
  • uv run python3 tests/dev_plan_table_test.py — 38 passed, 0 failed (7 new)
  • uv run python3 tests/ds_plan_table_test.py — 17 passed, 0 failed (2 new)
  • node tests/dev-run-driver.test.mjs — 35 passed, 0 failed (pre-existing, unmodified — proves no regression)
  • node tests/ds-run-driver.test.mjs — 28 passed, 0 failed (pre-existing, unmodified)
  • node tests/ds-grain-pause.test.mjs — 17 passed, 0 failed (pre-existing, unmodified)
  • py_compile on all 7 edited .py files
  • node --check on run-core.js, dev-task.js, ds-task.js
  • End-to-end compile smoke: synthetic PLAN.md fixtures (piped verify cell, em-dash test cell, qualified ds header) compiled via both dev_compile.py and ds_compile.py — emitted run.js node-checks clean and contains the correctly-parsed verify strings + the compiled REQUIRE_OUTPUTS_PRODUCED flag (false for dev, true for ds)
  • Re-read every edited template-literal in run-core.js/dev-task.js for unbalanced backticks (known incident: node --check doesn't catch this)

🤖 Generated with Claude Code

https://claude.ai/code/session_01J3NEAUUosUHyci4iFZBH2J

…ntion, ds prefix-tolerant header check, restore ds outputsProduced forcing function, mixed-level skip stamping, dep-cell phantom-dep guard, cleanup (v5.68.3)

Verified code-review findings on the pass-#9 shared compiled-runner core
(scripts/lib/plan_table_core.py + run-core.js + the dev/ds fragments/compilers):

1. plan_table_core.split_row / find_table split cell text on bare '|', so a Verify
   cell with a shell pipe (`pytest -q | tail`) or regex alternation
   (`grep -E 'foo|bar'`) shifted every later column. New _split_cells() is aware of
   escaped `\|` (unescaped to a literal pipe) and pipes inside backtick code spans;
   both split sites now share it. Regression tests added for shell-pipe, regex-
   alternation, escaped-pipe, and plain-row cases.

2. dev-task.js's testRequired() and dev_plan_table.py's now-deleted _TEST_NA
   diverged on em/en-dash cells ('—'/'-'): the parser treated them as N/A, the
   compiled gate's testRequired predicate did not, so a plan with a dash Failing
   Test cell compiled but could never satisfy its own gate. dev-task.js is now the
   SOLE owner of the predicate (comment cross-references the parser); the
   parser's dead test_required property/_TEST_NA are removed.

3. ds_plan_table.py's required-column check used exact `c not in header` while
   cell extraction is prefix-tolerant (dev already used the shared has_col()) —
   a qualified header like `Expected Output (per requirement)` was falsely
   rejected as missing. Now uses plan_table_core.has_col(), like dev.

4. The run-core extraction silently weakened ds's output-first self-report:
   outputsProduced went from schema-required + `=== true` to optional + `!== false`,
   muting a missing-field forcing function (gateProbe still independently checks
   output existence, so severity was low but real). Restored via a compiled config
   hole (__REQUIRE_OUTPUTS_PRODUCED__): ds_compile.py sets it True, dev_compile.py
   sets it False; run-core.js gates on it in both the schema `required` list and
   the pass/collect logic, without naming "ds" in the shared core.

5. plan_table_core.parse_deps swept the WHOLE Deps cell for digit-looking tokens,
   so free text like `T1 (needs config v2)` created a phantom dep on 'v2'. Now
   splits on commas/whitespace after stripping `after `/backticks and requires each
   piece to fullmatch a dep token; non-conforming residue is dropped from deps and,
   via an optional `violations` list threaded from both domain parsers, reported
   as a violation instead of silently becoming a dependency.

6. run-core.js's mixed-level branch (some tasks done, some todo) never stamped a
   state entry for the blind-skipped done tasks, so collect() under-reported
   tasksDone/over-reported tasksRemaining and scoreTable showed '·' for completed
   work. Now stamps the same skip-state shape the all-done branch uses.

7. Cleanup: dropped the dead `cfg.clearedRecheck` alias (no producer exists); the
   fragment-returned `recheckTrigger().atLevel` was always an echo of the `li`
   argument, so the core now supplies `atLevel: li` directly and dev-task.js no
   longer returns the field.

8. dev-plan-executable-guard.py's docstring wrongly said it's wired via
   dev-plan-reviewer frontmatter (that skill is read-only, never writes
   PLAN_REVIEWED.md) — corrected to dev-design/SKILL.md. Both guard hooks now
   import their parser lazily, inside validate_plan(), so the hundreds of
   unrelated Write/Edit hook firings per session skip the parser import.

Deferred (not attempted in this PR): extracting the duplicated compile driver/
guard shells into shared modules; dataflow (non-barrier) scheduling in run-core;
the pre-probe skip optimization; ds_compile._tier keyword-sniffing → a Tier
column; unifying the Python/JS toposort tie-breakers (a code comment now flags
that run-core.js's toposort must stay order-compatible with
plan_table_core.toposort_ids).

Version bump assumes PR #50 (5.68.1) and the pending round-2 PR (5.68.2) land
before this one; rebase/renumber if that ordering changes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J3NEAUUosUHyci4iFZBH2J
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant