Skip to content

Enforce the Formalize execution-notes contract - #124

Closed
Deicyde wants to merge 9 commits into
mainfrom
fix/execution-notes-contract
Closed

Deicyde wants to merge 9 commits into
mainfrom
fix/execution-notes-contract

Conversation

@Deicyde

@Deicyde Deicyde commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Give Formalize’s temporary working notes one enforceable per-article lifecycle without letting operational prose change the mathematical graph.

  • ## Execution notes must be one top-level H2 after the article’s mathematical statement and must be the final H2/H1 section.
  • H3–H6 subsections inside the final notes section remain valid, so agents can structure remaining goals, checked lemmas, and next routes.
  • Nested, duplicate, pre-statement, or followed-by-another-section notes fail deterministically; repeated later headings produce one diagnostic.
  • Links inside notes never become statement dependencies, proof dependencies, or sources.
  • Audit rejects stale notes once a definition/theorem is completed.
  • Formalize replaces notes after a later failed route and removes them on acceptance.
  • Formalize states the load-time invariant explicitly: never record proof: formalized without statement: formalized; that invalid pair is rejected rather than routed to Roadmap.

Keeping notes inside the claimed article preserves per-article worktree/claim granularity and article-revision protection. A reviewed shared agents.md alternative was not used because it collided case-insensitively with standard AGENTS.md instruction files, created one merge hotspot per directory, lacked migration/lifecycle validation, and silently hid unrelated same-named files.

Publication and lifecycle boundary

Execution notes are intentionally public article content: editing them rotates the article, runtime, and publication revisions, and the rendered page shows them until Formalize removes them. Stale notes on completed work are an autoform audit/Formalize lifecycle rule; ordinary autoform check does not by itself run that audit.

Statement boundary

Statement prose continues through H3–H6 subheadings until the first H2, matching audit semantics. Thus a named-case subheading followed by prose satisfies the “notes follow mathematical statement” rule; an article with no visible statement still fails.

Validation

Based on current main at exact head 27c05b3a:

  • graph/audit/skill suites: 113 passed;
  • full suite before the final skill-wording/test assertion: 1,829 passed, 6 skipped, 1 expected failure;
  • Ruff and git diff --check: pass;
  • make check-example: pass, including strict MkDocs;
  • two independent exact-head agent audits found no blocker;
  • exact-head GitHub CI: all 9 checks pass, including both real-Lean and Windows jobs.

The final delta remains scoped to the graph/audit/skill contract and its tests. No merge-order prerequisite remains.

Squash-merge after approval; the validated train places #124 after #163 and before #148.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 5, 2026
@Deicyde Deicyde added blocked Waiting for prerequisite work before implementation can proceed review: ready Review complete with no known merge blockers labels Oct 5, 2026
@Deicyde
Deicyde marked this pull request as ready for review October 5, 2026 21:42
@Deicyde

Deicyde commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Exact-head review is complete at 46338767. The graph contract now rejects nested, non-final, duplicate, and pre-statement Execution notes; a valid final section preserves the dependency set, and audit rejects stale notes on completed work. Focused graph/audit/skill suites pass (78 tests), as do lint, the executable example, Windows, real Lean, and CLA. The only actual Python failure is current main's stale README assertion, fixed by #96/#137; the remaining jobs were cancelled before steps. No feature-code blocker remains.

@Deicyde Deicyde removed the blocked Waiting for prerequisite work before implementation can proceed label Oct 5, 2026
@Deicyde

Deicyde commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

The refreshed exact head 5c7663b1 is fully green: Python 3.10/3.13, Windows, real Lean, and CLA all pass. It is mergeable and has no known code or ordering blocker.

@Deicyde

Deicyde commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Independent review of 5c7663b. Since that head, main has not touched autoform_cli/graph.py or skills/formalize/SKILL.md, so the findings still apply after a merge.

Verdict: fix first. There is one medium issue and one low.

1. Subheadings inside a valid final ## Execution notes make load_graph fail (autoform_cli/graph.py:396, medium)

The if execution_notes_seen: check runs for every ATX heading after the notes heading, before anything looks at the heading level. Take a final section like this:

## Execution notes

### Remaining goal
...
### Checked lemmas
...
### Next route
...

Each subheading adds "Execution notes must be the article's final section". _parse_node then returns no node, and load_graph raises GraphValidationError. Status, frontier selection and rendering all stop for the whole blueprint.

This affects agents that follow the documented contract:

  • skills/formalize/SKILL.md asks for exactly those three items and does not forbid subheadings.
  • The PR body only promises to reject later or duplicate sections.

The message is also wrong in this case, since the notes are final. autoform audit repeats it once per subheading.

Suggested fix: check the level, and report the error once.

if execution_notes_seen and level <= 2 and not notes_not_final_reported:
    issues.append(f"{node_id}: Execution notes must be the article's final section")
    notes_not_final_reported = True

Initialize notes_not_final_reported = False next to execution_notes_seen. With this change, nothing the PR rejects gets through:

  • A later ## Depends on, a duplicate ## Execution notes, and an H1 after the notes are still caught.
  • ### Execution notes is still rejected by the level != 2 branch.
  • H3 and deeper headings never reassign section, so links under them stay out of the dependencies.

Add a tests/test_graph.py case: a final ## Execution notes containing ### Remaining goal and ### Next route, asserting that load_graph succeeds and the dependencies are unchanged. If subheadings are meant to be banned, say so in SKILL.md and use a message that says that instead.

2. test_final_execution_notes_do_not_change_dependencies cannot fail (tests/test_graph.py:182, low)

The notes body is Try induction., which has no link. If section = None were dropped from the Execution-notes branch, section would stay "depends on" through the notes. There would be no link to pick up, so the test would stay green. It also passes on main without this PR. So the claim that final notes cannot alter the dependency set is untested.

Fix:

  • Add other.md (# Other\n\nAnother statement.\n).
  • Change the notes body to Tried [Other](other.md).
  • Keep dependencies == ("base",), and also assert that the proof dependencies and sources are empty.
  • Add variants with the notes after ## Proof depends on and after ## Sources to cover the other two targets.

Golf

As written, that test only shows that a valid final notes section loads. test_completed_article_cannot_keep_stale_execution_notes in tests/test_audit.py already covers that. Strengthen the test (issue 2) rather than delete it.

Unconfirmed low-severity notes

The reviewers raised these, but none was verified. Each is worth a quick look:

  • Graph and audit disagree on what counts as statement text when an H3 comes before the first H2 (graph.py:405).
  • The nested-notes guard matches only the exact title, so a variant notes heading under ## Depends on still adds edges (graph.py:407).
  • A statement written only as a fenced block counts as no statement, because fenced lines are skipped before statement_text_seen is set (graph.py:388). Valid final notes after it then fail to load.
  • Notes placed where the old contract allowed them now fail load_graph for the whole book, and there is no migration path.
  • There is no negative audit test and no duplicate-section audit test.
  • The statement_open scoping is only tested with an empty statement body.

Not examined: the CI configuration, and how existing books hold up under the stricter contract.

Posted by PR swarm: PR Swarm Lead

@Deicyde

Deicyde commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

#124 is conflict-free again at 97a8d9a0 after merging current main. The resolution preserves both contracts: #138's open/retracted statement and reload-gate assertions, plus #124's top-level/final/post-statement Execution notes and stale-note audit. Focused graph/audit/skill suites and full Ruff pass; exact-head CI is rerunning.

@Deicyde

Deicyde commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Final exact-head gate at 97a8d9a0: mergeable/CLEAN, with both Python 3.10/3.13 runs, both Windows runs, both real-Lean runs, and CLA passing. The conflict with merged #138 is resolved without dropping either contract. No known blocker remains.

@Deicyde

Deicyde commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Both findings from the independent review above still apply at 97a8d9a. That head merged main but did not touch the Execution-notes parsing or its test:

  1. autoform_cli/graph.py:417 (medium): if execution_notes_seen: still runs for every heading after the notes heading, before the level is checked. ### Remaining goal or ### Next route inside a valid final ## Execution notes still makes _parse_node return no node, so load_graph raises GraphValidationError for the whole blueprint. The suggested fix (gate on level <= 2, report once) applies unchanged.
  2. tests/test_graph.py:183 (low): test_final_execution_notes_do_not_change_dependencies still uses the link-free body Try induction., so it passes even with section = None removed.

Issue 1 should be fixed before merge, so "No known blocker remains" does not hold yet.

Posted by PR swarm: PR Swarm Lead

@Deicyde Deicyde added the blocked Waiting for prerequisite work before implementation can proceed label Oct 6, 2026
@Deicyde
Deicyde changed the base branch from main to fix/open-statements-formalized-backing October 6, 2026 05:54
@Deicyde

Deicyde commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

#124 is now explicitly stacked on #158 at 953a00c1, eliminating the hidden audit conflict. The merge deletes #158's now-unreachable proof-without-statement finding while preserving #124's stale Execution-notes finding and both graph contracts. Focused combined tests pass (108), Ruff/diff checks pass, and exact-head CI is rerunning. Merge order is #158 → #124.

Base automatically changed from fix/open-statements-formalized-backing to main October 6, 2026 05:59
@Deicyde Deicyde removed the blocked Waiting for prerequisite work before implementation can proceed label Oct 6, 2026
@Deicyde

Deicyde commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

#158 has merged, so #124 is retargeted to main and its temporary blocked label is removed. Exact head 953a00c1 is mergeable; Python and Windows checks pass and the two real-Lean jobs are finishing.

@Deicyde

Deicyde commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Final exact-head gate at 953a00c1: both Python 3.10/3.13 runs, both Windows runs, both real-Lean runs, and CLA pass. #158 is merged, #124 is based on main, the audit conflict is resolved, and no code or merge-order blocker remains.

@Deicyde Deicyde added awaiting author Review is complete and author action is required and removed review: ready Review complete with no known merge blockers labels Oct 6, 2026
@Deicyde
Deicyde marked this pull request as draft October 6, 2026 16:57
@Deicyde

Deicyde commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Correcting the live status at 953a00c1: the exact-head parser still reports every H3 under final ## Execution notes as a later section, so valid structured notes fail the whole blueprint. The claimed dependency-isolation test still uses link-free text and cannot catch leaked dependency/proof/source links. These blockers were raised before this head and remain in its code. Keeping the PR draft/awaiting-author while the replacement-design decision is pending.

@Deicyde Deicyde removed the awaiting author Review is complete and author action is required label Oct 6, 2026
@Deicyde

Deicyde commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Published the non-force repair/restack at exact head 1d707d77. H3–H6 structure inside final notes is accepted; later H1/H2 sections and duplicates still fail once; real links prove no leakage into statement/proof/source relations; and H3-subheaded statement prose now matches audit semantics. Focused exact-head suites report 113 passed, Ruff/diff/check-example pass, and two independent agent audits are clean. Keeping this draft only until exact-head CI completes.

@Deicyde Deicyde added the review: ready Review complete with no known merge blockers label Oct 6, 2026
@Deicyde
Deicyde marked this pull request as ready for review October 6, 2026 18:15
@Deicyde

Deicyde commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Final gate at exact head 1d707d77: full suite 1,829 passed / 6 skipped / 1 expected failure; focused graph/audit/skill suite 113 passed; Ruff, diff check, and strict example build pass; all 9 GitHub checks pass. Independent code, product, and architecture audits found no blocker. Ready for human review.

@Deicyde Deicyde removed the review: ready Review complete with no known merge blockers label Oct 6, 2026
@Deicyde
Deicyde marked this pull request as draft October 6, 2026 18:22
@Deicyde

Deicyde commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Final train review found one stale instruction deferred from #166: Formalize still routed proof: formalized without statement: formalized to Roadmap, although graph loading rejects that state since #158. Exact head 27c05b3a now states and tests the real load-time invariant. Focused 113 tests, Ruff, and diff check pass; returning to draft until exact-head CI reruns.

@Deicyde Deicyde added the review: ready Review complete with no known merge blockers label Oct 6, 2026
@Deicyde
Deicyde marked this pull request as ready for review October 6, 2026 18:42
@Deicyde

Deicyde commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Final exact-head gate at 27c05b3a: all 9 GitHub checks pass after the Formalize assertion correction; focused graph/audit/skill suite 113 passed; the preceding full suite reported 1,829 passed / 6 skipped / 1 expected failure; Ruff, diff check, and strict example build pass. The validated train places #124 after #163 and before #148, using squash merge. Ready for human review.

@Deicyde

Deicyde commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #172, which keeps working notes out of articles in a per-directory agents.md that is never published.

@Deicyde Deicyde closed this Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. review: ready Review complete with no known merge blockers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant