refactor(engine): engine-hygiene — behavior-preserving cleanup + wire milestone-relations (2 tasks)#156
Merged
Merged
Conversation
…hoist, snapshot-hash + milestone-resolve helpers, static-regex hoist (hygiene-bundle) Four value-dense cleanups a read-only sweep surfaced, bundled into one fence/twin/pin ceremony. Behavior byte-identical — the ~3600-test fence is the guard; 4 new structural tests pin each change red→green. #1 cmd_check perf — stop re-reading components.toml per bound task: - drop the dead recompute `_arch = _archived_task_slugs(state)` (the outer `archived_slugs` already held it). - hoist `_components(root)`/`_contracts(root)` to once-per-invocation locals (mirrors cmd_components), thread the registry into `_task_component(root, slug, comps)` AND `_component_findings`'s own per-task loop. The result is O(1) reads regardless of bound-task count — pinned by a spy asserting the `_components` call count is equal at 2 vs 4 bound tasks. #2 one snapshot-hash reader — `_snapshot_hash(path) -> str | None` replaces 5 duplicated inline `json.loads(p.read_text()).get("hash")` + try/except copies whose caught tuples had drifted apart (3 sites narrower, 2 broader). Unified to the broad safe set; never raises (fail-open, an unreadable pin reads as absent). The `_consumer_stale_guard` except→return site converts cleanly — the following `if live is not None` guard is a no-op on None with nothing after it. #5 one milestone resolver — `_resolve_milestone(state, slug)` (twin of `_resolve_task`) replaces the byte-identical bare `_die("unknown_milestone")` boilerplate at 4 sites. The `milestone and …` / `entry is None` / `else` variant sites keep their own distinct wording (milestone_resolve_message_drift reject). #3 static-regex hoist — `taskdoc._HEADING_RE` compiles the §-heading pattern once at module load instead of per `_phase_spans` call (the busiest task-doc read path). Behaviour byte-identical. Task A of the engine-hygiene milestone (from an Explore sweep; user-picked scope). Red/green: test_hygiene_bundle (8 tests). Synced ×4 add.py + ×4 add_engine/ taskdoc.py twins; ENGINE_MD5→c3d61a86, ENGINE_PKG_MD5→265dd143; SEAMS _declared_scope re-pinned 5723→5734. Sweep verdicts SKIP #6/#7/#8. author: Tin Dang <tindang.ht97@gmail.com>
…re-milestone-relations)
The sweep found `_milestone_relations` parsed a MILESTONE.md's
depends-on/extends/relates-to header edges but was called by NO command (only its
test) — a half-built feature, while the task-level twin is fully wired. This
finishes it.
New `_milestone_relations_health(root, state)` mirrors `_relations_health`:
returns `{mslug, relation, target, kind}` findings — `dangling` when an edge names
an unknown milestone, `self_relation` when it names itself. All three edge kinds
are validated (milestone edges are cross-milestone legibility, never a build DAG,
so unlike the task twin's depends_on they don't belong to any schedule).
Surfaced ADVISORY (never red, never blocks a gate):
- cmd_check: one WARN per finding (feeds `warnings`, never `checks`/`failed`),
reading the hyphen label the human wrote (`relates-to 'ghost' which is not a
milestone (dangling)`).
- cmd_status: a one-line count `milestone-relations: N dangling · M self — run
add.py check` beside the existing task `relations:` line, silent when clean,
human branch only (never --json/--brief).
This also wires the cmd_check per-finding detail the status line's "run add.py
check" already promised — so milestone-relations is now more completely surfaced
than the task twin.
Task B of the engine-hygiene milestone (#4 from the read-only sweep; user chose
"wire it up"). Red/green: test_wire_milestone_relations (6 tests). Synced ×4 add.py
twins; ENGINE_MD5→d7079f8d (no add_engine change → ENGINE_PKG_MD5 unchanged);
SEAMS _declared_scope re-pinned 5734→5778.
author: Tin Dang <tindang.ht97@gmail.com>
Replace the MILESTONE.md scaffold placeholders with the real milestone record now that both tasks are built, gated PASS, and committed: - Scope: the confirmed #1+#2+#3+#5 behavior-preserving cleanup + #4 finish-wire, with the deferred #6/#7/#8 named as the anti-scope-creep Out list. - Tasks: hygiene-bundle + wire-milestone-relations, both checked done. - Exit criteria: the three observable outcomes (cmd_check O(1), shared snapshot-hash/milestone-resolve helpers, advisory milestone-relation surface). - Close ship-review: ship-by-domain (tooling only; skill/book untouched), the per-task gate/tests/fence evidence rows, and the goal-met mapping back to each exit criterion. Full fence last green at 3626 passed / 0 failed. - Release steps: commits landed; push + PR + merge-on-green next; the paid WM1 re-measure waits for merge (human-authorized "wait for engine-hygiene"). Doc-only — no engine change, so no ENGINE_MD5 / SEAMS re-pin. author: Tin Dang <tindang.ht97@gmail.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| with redirect_stdout(io.StringIO()), redirect_stderr(io.StringIO()): | ||
| try: | ||
| add.main(list(argv)) | ||
| except SystemExit: |
| with redirect_stdout(io.StringIO()), redirect_stderr(io.StringIO()): | ||
| try: | ||
| add.main(["check"]) | ||
| except SystemExit: |
| with redirect_stdout(io.StringIO()), redirect_stderr(io.StringIO()): | ||
| try: | ||
| add.main(list(argv)) | ||
| except SystemExit: |
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
2 participants
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.
What
Engine-hygiene milestone (2 tasks) — value-dense cleanup a read-only engine sweep found, plus finishing a half-built feature. No behavior change to any existing gate.
cfb8fe0— refactor(engine): behavior-preserving perf/dedup cleanup (hygiene-bundle)Findings #1+#2+#3+#5 from the sweep, all behavior-preserving:
cmd_checkre-read + re-parsedcomponents.toml/contracts off disk per task (+ a dead second_archived_task_slugsrecompute). Hoisted to O(1) likecmd_componentsalready does —_components/_contractslifted out of the per-task loop, dead_archremoved,_task_component(comps=)and_component_findings(_reg)threaded so no residual per-task disk read remains.json.loads(<snap>.read_text()).get("hash")was duplicated 5× with inconsistent except tuples. Extracted_snapshot_hash, unified to the broader tuple.taskdoc.pyrecompiled a static^##\s*(\d+)\s*·regex on every_phase_spanscall (hot, 21+ callers). Hoisted to a module const.unknown_milestoneresolve boilerplate 4× →_resolve_milestone(mirrors_resolve_task).0dddefd— feat(engine): wire milestone-relations health into check + status (wire-milestone-relations)Finding #4:
_milestone_relationsparsed a MILESTONE.md's depends-on/extends/relates-to edges but was wired to no command (only its test) — while the task-level twin is fully wired. New_milestone_relations_healthmirrors_relations_health, surfaced advisory (never red, never blocks a gate):cmd_check: one WARN per finding (dangling / self edge) — feedswarnings, neverchecks.cmd_status: a one-line countmilestone-relations: N dangling · M self, silent when clean.36b5c15— docs(method): fill engine-hygiene milestone close ship-reviewTesting
test_hygiene_bundle.py(8, incl. an O(1) call-count spy comparing N=2 vs N=4 tasks) ·test_wire_milestone_relations.py(6)..add/SEAMS.mdline-pins migrated.Deferred (Out of scope)
Sweep findings #6 (no_project boilerplate — too small), #7 (long fns — verbose not broken), #8 (13 broad
except— all intentional documented fail-open).