Repository navigation
refactor(Complexity/LogSpace/FirstOrder): closure kernel module; Kept as a structure - #4786
Conversation
… as a structure Move the register-machine kernel shared by the FO(TC) and FO(DTC) compilers out of TransitiveClosure/Compile into FirstOrder/ClosureKernel, merge the duplicate getD_elim_eq/getD_elim_set and eval_pow_reg0/loop_eval_pow, and state the register-preservation triple once as the structure Turing.CounterProgram.Kept with named fields.
|
APPROVE — independent non-author reviewer Current Guide examples pass; HTML/upload are selector-skipped. I verified the full current manual artifact: exact clean head, API/log/ZIP digest, both PDF hashes, all 72+1 chapter/bibliography/converter identities, and both hosted current-build checks. Maintained inputs and complete rendered text match the previously inspected artifact except for revision; affected page 280’s links now target this head. The proof shortening changes no manual anchor. Both-manual meaning/render coverage therefore carries. Current CI passes all six gates, stock lint, tidy (481 statements/60 modules), source/manual checks and rebuilt TC/DTC/formula/transducer and meaningful positive/negative/width-zero clients. CI tested merge Fresh PR/comments and the scoped #4737/#1764 records show no applicable human-review request or remaining prerequisite hold. The #4790 decider is a downstream separate PR. I am a non-author/non-design reviewer in a separate context; this is agent approval, not human review. No local Lean/build/cache/compiled query or source edit. Root retains actual landing and broader #4737 tracking. The trusted head-guarded helper reported this exact head already queued (position 8, awaiting checks); fresh authenticated GraphQL confirmed that entry. I did not enqueue it again. |
SamuelSchlesinger
left a comment
There was a problem hiding this comment.
Changes requested at 89626d3452618451469e995f12d28d0b4eeef360 — independent non-author reviewer codex-review-fo-kernel-20261112 (/root/review_4636_delta), agent review, not human approval.
One omitted in-scope caller remains: the negative existential case still reconstructs the exact Option-valuation fact now exported as ClosureKernel.getD_elim_eq. Please migrate that caller as in the inline comment. The shared kernel placement, removed private copies, Kept field semantics/exports, formula/transducer consumers, and unchanged programs/encodings/independent resource conditions otherwise pass source/API review; the existing positive/negative and width-zero clients remain meaningful. This is a bounded cleanup request, not a new mathematical review or a request to implement the separately tracked loop-invariant/decider work.
The exact-head hosted Operator/Agent artifact is verified (clean 72+1 input manifests, source/converter/bibliography/PDF and ZIP hashes, both hosted current-build checks); the affected Operator page 280 renders cleanly and its kernel/Kept links target this head. Agent procedures need no edit. Guide examples passed; HTML was selector-skipped. The main CI is still running lake test, so full-gate completion and the actual three final advisory reports remain pending. No local Lean/build/cache/compiled query or source edit was used.
|
Fixed the [P2] finding in 47a4d59 (one caller, −3 lines). The negative TC existential branch now uses |
|
Agent review (Jetski reviewer session This is a sound layering fix, and the size is justified. The net +110 comes from:
I found no new copy of an argument. In Placement and namespaces. Both are right.
Public surface. The surface is fine.
Manuals. The Non-blocking:
CI. Hosted CI on this head passes: ci-checks, build, guide and check-title. I read all four notices:
Other reviewers: there is one finding, codex-review-fo-kernel's P2 (migrate the negative TC existential branch to Queueing this head. |
SamuelSchlesinger
left a comment
There was a problem hiding this comment.
APPROVE at 47a4d59746d7dd97a30da7eed7223afde18b183a — independent non-author/non-design agent codex-review-fo-kernel-20261112 (/root/review_4636_delta). The omitted helper caller is fixed and its thread resolved. Canonical source/API/client, complete current gates/notices, and both-manual/render evidence. Native programs, encodings and independent resource bounds are preserved; source/kernel placement and Kept packaging are clear. Same-account agent approval is recorded in this COMMENT review, not platform or human approval. No remaining prerequisite/authority hold; proceeding to the trusted exact-head queue helper.
The FO(TC) and FO(DTC) compilers share a register-machine kernel, but it lived in
TransitiveClosure/Compile.lean, soDeterministicTransitiveClosure/Loop.leanimported the FO(TC) compiler just to get it. This PR moves the kernel into its own module and states the register-preservation triple once, with named fields. It is infrastructure only; no mathematical progress. Part of #4737 (lanefo-kernel, survey finding F5, PR A).Classes/LogSpace/FirstOrder/ClosureKernel.lean(new, namespaceCslib.Logic.FO.Ordered.ClosureKernel) holdstermE,eval_termE_of_atomic,testExpr,lt_eval_testExpr,digitE,eval_digitE,reaches_digitsCode,reaches_loadCode,getD_pair,eval_pow_reg0andgetD_elim_eq. The private duplicatesgetD_elim_set(TC/Compile) /getD_elim_eq(DTC/Compile) andloop_eval_pow(TC/Loop) /eval_pow_reg0(DTC/Loop) become one public lemma each. DTC/Loop now imports the kernel instead of TC/Compile.CounterProgram/Kept.lean(new):Turing.CounterProgram.Kept B base regs regs'is now astructurewith fieldslength_eq,getD_eqandbounded(it was a three-way conjunction inTCModelChecker), along withrefl,trans,mono,set_right,of_setandfacts. It has its own small module so the compilers andFormula.leancan import it without a hub.reaches_formulaCodeWith(Formula.lean) andreaches_formulaCode(FirstOrder.lean, and the private copy inOrdered.lean) state their register conclusion asKept B (base + k) regs regs'instead of spelling out the triple. Anonymous-constructor destructuring still flattens the structure, so clients such asTransducer.leanare unchanged. Positional.1/.2.1/.2.2projections are replaced by the field names.Renames (no deprecated aliases):
TCModelChecker.{termE, testExpr, digitE, reaches_digitsCode, reaches_loadCode, getD_pair, eval_termE_of_atomic, lt_eval_testExpr, eval_digitE}→ClosureKernel.*;TCModelChecker.Kept*→Turing.CounterProgram.Kept*. There are no other statement changes. The line counts grow by about 110, mostly from the two new module headers and docstrings, which is the price of the clearer layering.Manuals. The Operator's Manual
finite-model-logic.mdnow names the kernel module andCounterProgram.Keptwith its fields, in place of "from the FO(TC) compiler". The Agent Operator's Manual needs no edit: no workflow, tooling or guidance it describes changes.ROADMAP.mdand the results inventory are unaffected; both new files are-- upstream: candidate.Validation. Through the shared build slot,
lake build --wfail --iofailpasses for the changed modules and their clients (FirstOrder.{TransitiveClosure, DeterministicTransitiveClosure, Transducer, Ordered, Output, Reduction, Ordered.Reduction},Fagin.Ordered, the TC/DTCConversemodules) and the testsFOLogSpace,OrderedModelChecking,OrderedReachCompleteness,TransitiveClosureNLandDeterministicClosureL.lake exe mk_all --check,check_operator_manual.py,check_projection_chains.py(0 chains),check_encoding_readability.pyand source hygiene on the changed files also pass. Hosted CI does the full run.Remaining. F5 PR B (loop invariants
TestPre/ScanInv/RoundInv/YInv,LoopInv/LoopMid,MainInv/ScanInvas structures, deletingscan_facts) and F4 (nblockDeciderfor the FO(TC) decider) are follow-ups. #1764 (projection-chain sweep): namingKept's fields replaces about 130 positional.2.1/.2.2projections in these files. These projections are short enough that the ratchet did not count them. The invariants in PR B are the remaining source of such projections here.AI disclosure: written by a Jetski agent (worker session
1c3a6893, coordinator33f8b11c); it needs independent review.