mcp/SKILL.md: add pitfalls and tactics from s2n-bignum GHASH proof work#164
Merged
jrh13 merged 1 commit intojrh13:masterfrom Apr 13, 2026
Merged
mcp/SKILL.md: add pitfalls and tactics from s2n-bignum GHASH proof work#164jrh13 merged 1 commit intojrh13:masterfrom
jrh13 merged 1 commit intojrh13:masterfrom
Conversation
- Fix ONCE_REWRITE_TAC description: 'one pass, all topmost matches' not 'only once' - Add GEN_REWRITE_TAC examples with RAND_CONV and LAND_CONV conversionals - Note that ARITH_TAC ignores hypotheses (goal only) - Add SUBGOAL_THEN variants: MP_TAC, SUBST1_TAC, STRIP_ASSUME_TAC - Add ABBREV_TAC / EXPAND_TAC pair - Add pitfalls: SUBST1_TAC silent success, THEN vs THENL after SUBST1_TAC, ONCE_REWRITE_TAC all-matches semantics, ARITH_TAC ignoring hypotheses Co-authored-by: Kiro (Claude Opus 4.6) <kiro@amazon.com>
Owner
|
Thank you, this is a clear improvement - I will merge :-) |
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.
Motivation
These additions come from sessions of formal GHASH/POLYVAL proof work in s2n-bignum, where each pitfall was hit at least once and caused wasted tactic attempts + backtracking cycles.
The SKILL.md is loaded as LLM context at the start of every HOL Light MCP session, so these corrections directly change what the LLM tries on the first attempt rather than the third.
What changed
Fixes to existing content:
ONCE_REWRITE_TACdescription: "Rewrite only once" → "Rewrite one pass, all topmost matches (not just one match!)". The old description suggests one match; the actual behavior rewrites ALL topmost matching subterms in a single pass. For targeted single-occurrence rewriting,GEN_REWRITE_TACwith conversionals is needed.ARITH_TACdescription: added "(goal only, ignores hypotheses)". Without this, the natural first attempt on a goal with needed facts in the hypotheses isARITH_TAC, which fails silently. The fix isUNDISCH_TACfirst.New tactic entries:
GEN_REWRITE_TAC (RAND_CONV)andGEN_REWRITE_TAC (LAND_CONV o ONCE_DEPTH_CONV)— essential for targeted rewriting whenONCE_REWRITE_TACrewrites too many occurrencesSUBGOAL_THENwithMP_TAC,SUBST1_TAC,STRIP_ASSUME_TAC— the doc only hadASSUME_TACABBREV_TAC/EXPAND_TAC— fundamental to s2n-bignum proofs, completely absent beforeNew pitfall entries:
SUBST1_TACsilently succeeds even when the LHS doesn't appear in the goal, makingFIRST_ASSUM(fun th -> SUBST1_TAC(SYM th))unreliable. UseEXPAND_TACinstead.THENvsTHENLafterSUBGOAL_THEN ... SUBST1_TAC—THENapplies the equality proof to ALL subgoals including the main goal after substitution.Why these specific additions
Each one prevents a concrete failure mode:
ARITH_TACignores hypotheses — Without this note, when seeingm + n < 2 EXP 64with bounds in hypotheses, the first instinct isARITH_TAC. It fails. ThenASM_ARITH_TAC. It hangs (30 assumptions withvalterms). Then finallyUNDISCH_TACtwice +ARITH_TAC. Three wasted round trips. With the note, go straight toUNDISCH_TAC.ONCE_REWRITE_TACsemantics — When commuting oneword_xorin a goal with five of them, the old description leads toONCE_REWRITE_TAC[WORD_XOR_SYM]. All five get commuted. The newGEN_REWRITE_TACexamples show the correct syntax.ABBREV_TAC/EXPAND_TAC— In ARM proofs, register values are named constantly. Without these, the brokenFIRST_ASSUM(fun th -> SUBST1_TAC(SYM th))pattern gets used. TheSUBST1_TACpitfall entry warns away from that.SUBGOAL_THENvariants — Defaulting toASSUME_TACfor everything means doingSUBGOAL_THEN eq ASSUME_TAC THEN ASM_REWRITE_TAC[]in two steps instead of the directSUBGOAL_THEN eq SUBST_ALL_TAC.