check_system advises on key shape, and the shape rules design is closed out (D64, step 5)

README.md

@@ -21,7 +21,7 @@ Everything else runs without them. Python 3.11 or newer.
 | --- | --- | --- |
 | `./gen_bittings.py` | `sfic-gen-bittings` | Random bittings from a parity pattern, or with any parity if you give none, optionally staying away from existing bittings, and following the [key shape rules](#key-shape-rules). |
 | `./check_bittings.py` | `sfic-check-bittings` | Quick check of a few `NAME=BITTING` values: format, parity, adjacent-cut limit, pairwise closeness, with advice on the [key shape rules](#key-shape-rules). |
-| `./check_system.py` | `sfic-check-system` | Whole-scheme check of a system file: per-key rules, duplicates, closeness, operating-set sizes, cross-operation, residual risk. |
+| `./check_system.py` | `sfic-check-system` | Whole-scheme check of a system file: per-key rules, duplicates, closeness, operating-set sizes, cross-operation, residual risk, and advice on the [key shape rules](#key-shape-rules). |
 | `./solve_system.py` | `sfic-solve-system` | Fills in the `null` bittings of a system file by random search plus hill climbing, then runs the full check. |
 | `./pin_system.py` | `sfic-pin-system` | Prints the pinning chart of every core of a system file that sets `pinning`, in the layout `check_charts` reads: the pins to put in each chamber, with `--draw` a drawing of the stacks and with `--pdf` one PDF to print. |
 | `./check_charts.py` | `sfic-check-charts` | Checks that the tools' pinning rules reproduce pinning charts from your keying software (`.txt` files kept outside this repository). Reports positions only, never key data. |
@@ -121,7 +121,11 @@ The reasons, the numbers behind the defaults and what each costs in key space ar
   the `masters` of any core). The system file sets them in an optional `shape` object, with
   the rule names of the table above and `null` to turn one off (see
   [System file format](#system-file-format)).
-- `check_system` does not use the rules yet.
+- `check_system.py` prints the same advice for the keys and control keys of a system file, in
+  a "Key shape" section, using the file's `shape` settings. Decoded unit keys and retired keys
+  are left out (real keys already cut cannot be changed, and about half of any real set breaks
+  `max_run` = 1 without a pattern), and, as in `check_bittings`, it does not count as a problem
+  or change the exit status.
 
 ```bash
 ./gen_bittings.py --pins 7 -n 3 --master          # three masters, no parity pattern

TODO.md

@@ -2,6 +2,14 @@
 
 ## Ideas for later
 
+- The solver's objective still rewards change keys that hold a master's cut, and keys far from
+  their master, because both shrink the count of valid false keys: after the shape rules a
+  change key shares a cut with a master at 26.8% of positions against 20% by chance
+  (docs/designs/key-shape-rules.md, "What building it showed"). Changing what is scored needs
+  its own measurement and decision (D6).
+- Exact reachability bounds for the shape rules (the parity pattern and how the rules combine),
+  so that a rule set no key can meet is refused when the file is read, not when the search
+  gives up (the bounds look at each rule alone).
 - A solver for pinnable answers among more than three unknown keys that share cores
   (a system with every key blank): joint moves, or splitting the chain of cores
   (docs/designs/pinnable-solving.md, "Building pinnable keys exactly").

docs/decisions.md

@@ -705,3 +705,17 @@ choose and never silently relax; the exact counts and the risk figures are uncha
 without `shape`, which is deliberate (D6). The design is in its own document and has been accepted.
 
 Detail: [designs/key-shape-rules.md](designs/key-shape-rules.md).
+
+## D65. A key with no bitting that follows the shape rules is left null
+
+When the solver cannot find a bitting for a key that follows the file's shape rules (a tight
+rule set, or a thin pinnable set), it does not relax a rule. It leaves the key `null` in the
+written file, prints a `NOT SOLVED` line that says which rules turned the draws down (before
+`Wrote` and again last, as in D59), and skips the full check, which would reject the null.
+Keys built together name the key that broke the rules in most draws, and the others say they
+waited for it. The file's `shape` object, or a known key, is the way out. `check_system` and
+`check_bittings` only advise about existing keys, in `SHAPE` lines that change neither the
+exit status nor the counts.
+
+Detail: [design.md, "The code"](design.md#the-code) and
+[designs/key-shape-rules.md](designs/key-shape-rules.md).

docs/design.md

@@ -159,6 +159,17 @@ by a separate bounded search for each sample. For keys built together, the `NOT
 reason names a key only if it broke a rule in at least 80% of the draws (any random key
 breaks one now and then); the others say they were built together with it.
 
+**Shape advice in the checker** (D64). `check_system` and `check_bittings` print the keys that
+break a rule as `SHAPE` lines, with pin positions and never cuts, in a "Key shape" section that
+is left out when every rule is off. They are advice: they change neither the exit status nor
+the count of problems or warnings, and decoded unit keys and retired keys are not checked.
+Retired keys are history, which is also why D11 exempts them from parity and the adjacent-cut
+limit. Decoded unit keys are exempt from the `SHAPE` lines only (parity and the adjacent-cut
+limit still apply to them): the vendor cut them, so there is nothing to change, and without a
+parity pattern about half of any real set breaks `max_run` = 1, so anything louder would be
+noise. The rules never enter the exact counts or the
+residual-risk figures: they filter what the tools choose and leave what the tools count alone.
+
 ## The key space
 
 The rules that decide which bittings can be cut live in one frozen object,

docs/designs/key-shape-rules.md

@@ -1,11 +1,13 @@
 # Rules for the shape of a generated key
 
-Status: Accepted
-
-This document proposes a small set of rules about what a key's cuts look like (not
-which keys can be pinned together) and a way to apply them to the keys the tools
-generate: the solver's unknown keys and `gen_bittings`' output. It is written before
-any code, and accepted by the maintainer ([D64 in decisions.md](../decisions.md)). The numbers are measured on fake
+Status: Implemented
+
+This document describes a small set of rules about what a key's cuts look like (not
+which keys can be pinned together) and the way they are applied to the keys the tools
+generate: the solver's unknown keys and `gen_bittings`' output. It was written before
+any code, accepted by the maintainer and built in the five steps of its plan
+([D64 and D65 in decisions.md](../decisions.md)); "What building it showed" says what the
+build added. The numbers are measured on fake
 systems by throwaway scripts that are not in the repository, and are illustrations
 of scale, not claims about any real system.
 
@@ -128,10 +130,11 @@ only the 7% of bittings with three in a row and leaves the pairs, which is the c
   NOT SOLVED line names the key and the rules, before `Wrote ...` and again last.
 - **Keys that already exist** are not changed. `check_system` and `check_bittings` print
   the ones that break a rule as an advisory `SHAPE` line, with positions only and never
-  values. It does not change the exit status or the count of warnings. Decoded unit
-  keys and retired keys are left out (the vendor cut the first, and the second are history,
-  as for parity and MACS in D11), and without a pattern about half of any real set breaks
-  `max_run` = 1, so anything louder would be noise.
+  values. It does not change the exit status or the count of warnings. Retired keys are
+  left out because they are history, which is also why D11 exempts them from parity and MACS.
+  Decoded unit keys are left out of the `SHAPE` lines only (parity and MACS still apply to
+  them): the vendor cut them, so there is nothing to change, and without a pattern about half
+  of any real set breaks `max_run` = 1, so anything louder would be noise.
 
 ## Why not part of KeySpace
 
@@ -206,23 +209,36 @@ Separate commits, each with tests:
    a range test (not exact figures) on the share of keys with equal neighbours, run with and
    without a pattern; and the "no candidate" report.
 5. The advisory lines in `check_system`; README, `docs/design.md` and the file-format notes
-   in the same commit as the behaviour they describe.
+   in the same commit as the behaviour they describe. (The README's `shape` row and the
+   solver's paragraph came with step 4, and the generator's flags with step 3, as the
+   behaviour did.)
 
 A refactor, if one is needed to share the check between the tools, is its own commit and
 shows seeded output unchanged for files that turn every rule off.
 
-## What cannot be known yet
+## What building it showed
+
+These were open questions when the design was accepted.
 
 - Whether the rules can be met together. The reachability bounds look at each rule alone and
   ignore the parity pattern, so a combination can pass them and still have no key: 3 pins
   with `--master` and the default `max_step` of 5 need a span of 6, which no key of three
   cuts has unless it is monotone, which the default rules forbid. The tool then runs to its
   attempt limit and says so, naming the rule that turned down most of the draws, and does
   not relax anything. Three pins is not a real key, so the bounds were left simple.
-- How many pinnable candidates survive the rules in a system with many known keys: a unit
-  master that must also take a decoded unit's neighbours has a thin pinnable set already,
-  and the first run on such a file will say whether `master_min_span` = 6 is too strict.
-- Whether single-position moves still connect the valid keys once `max_run` = 1 applies
-  without a pattern; I expect so (the share kept is 46%) and the search tests will show.
-- Whether holding and spreading, the objective's bias, still produces odd keys once the
-  rules are in. That is the measurement to make after step 4, with the same scripts.
+- **A thin pinnable set can leave no key that follows the rules.** A unit master among seven
+  random decoded units (a fake system) had 24 pinnable masters, and all 24 ended in two equal
+  cuts, so with the defaults the solver reports `NO pinnable bitting follows the shape rules`
+  and leaves the key `null`. It is reported, not repaired: the way out is the `shape` object
+  (for example `max_run: null`) or a known key. How often a real system hits this depends on
+  how many unit keys are decoded, and cannot be known from fake systems. A key built together
+  with the one that has none is left null too, and says whose rules it waited for (D65).
+- **Single-position moves still connect the valid keys.** The hill climb and the draws never
+  got stuck in the 100 seeded solves measured (60 with a parity pattern, 40 with pinning and
+  none) or in the tests; no solve ended in `NOT SOLVED` for want of a shaped key.
+- **Holding and spreading, the objective's bias, is still there.** After the rules, a change
+  key shares a cut with a master at 26.8% of positions (parity pattern, 60 seeds), against
+  24.7% before and 20% by chance, so the solver still likes to hold cuts. The rules removed
+  the shapes it produced (equal neighbours 64% to 0%, a master's mean cut about 1 to 4.2,
+  total variation 12.1 to 14.8 against 15.0 for a uniform draw) and did not touch the cause.
+  Changing what is scored needs its own measurement and decision (D6); it is in TODO.md.

sfic_solver/check_system.py

@@ -4,6 +4,9 @@
 Reads a JSON description of the system and reports:
   * per-key problems (parity and adjacent-cut limit for keys and control keys;
     duplicates across all keys)
+  * advice on the shape of each key (the file's shape rules: equal adjacent cuts, a depth used
+    too often, a key that only goes one way, a master that is too narrow); positions only, not
+    counted as problems, and decoded unit keys and retired keys are left out
   * pairs of keys that are too close
   * for each core type, how many keys operate it (intended + false keys)
   * known keys that would operate a core they are NOT meant to operate
@@ -25,6 +28,9 @@
                    the pin count if smaller);
                    pairs involving unit keys are skipped unless
                    close_check_units is true (cross-operation is the real test)
+    shape          optional object of key shape rules (max_run, max_same_depth,
+                   forbid_monotone, master_min_span, min_total_variation; null turns one
+                   off); see the README
     unit_prefix    key-name prefix for unit keys (default "unit:")
     unit_count     total number of units, to estimate undecoded-key risk
     keys           {name: bitting} operating keys, including decoded unit keys
@@ -144,6 +150,16 @@ def main(argv=None):
             problems += 1
     print("done")
 
+    if not cfg.shape.is_off():
+        print("\n== Key shape: advice only (decoded unit keys and retired keys are not "
+              "checked) ==")
+        masters = cfg.master_keys
+        checked = {**{n: c for n, c in keys.items() if not n.startswith(unit_prefix)}, **control}
+        advice = [f"SHAPE     {name}: {cfg.shape.describe(rule, pins)}"
+                  for name, cuts in checked.items()
+                  for rule, pins in cfg.shape.violations(cuts, name in masters)]
+        print("\n".join(capped(advice) or ["none"]))
+
     print(f"\n== Closeness (non-unit keys differing in fewer than {min_diff} positions) ==")
     scope = {n: c for n, c in everything.items()
              if cfg.close_check_units or not n.startswith(unit_prefix)}

sfic_solver/config.py

@@ -45,6 +45,11 @@ class Config:
                                                               #  covers_units}
     shape: model.ShapeRules = field(default_factory=model.ShapeRules)   # the file's `shape`
 
+    @property
+    def master_keys(self):
+        """The names of the keys that are masters: those in the `masters` of any core."""
+        return {m for core in self.cores for m in core["masters"]}
+
 
 def _no_duplicate_keys(pairs):
     seen = {}

sfic_solver/model.py

@@ -264,6 +264,12 @@ def for_space(cls, space, **fields):
         fields.setdefault("master_min_span", span if span >= 1 else None)
         return cls(**fields)
 
+    def is_off(self):
+        """True if every rule is off, so every key passes."""
+        return (self.max_run is None and self.max_same_depth is None
+                and not self.forbid_monotone and self.master_min_span is None
+                and self.min_total_variation is None)
+
     def describe(self, rule, pins):
         """A phrase for a broken rule and its pins (from `violations`), saying where and
         never what the cuts are."""

sfic_solver/solve_system.py

@@ -59,8 +59,6 @@
                                # position lists up to 10 ** this digit combinations
 MAX_SHAPE_REJECTS = 20_000     # draws turned down by the shape rules before a key is given up
                                # on (at least 50 for each candidate asked for)
-SHAPE_OFF = model.ShapeRules(max_run=None, max_same_depth=None, forbid_monotone=False,
-                             master_min_span=None, min_total_variation=None)
 UNPINNABLE_WEIGHT = 1.0        # one undecoded unit key that cannot take the master counts as
                                # one expected cross-operation; with the scale of the two terms
                                # it makes avoiding rekeyed unit cores the primary goal
@@ -147,8 +145,8 @@ def __init__(self, cfg):
         # Shape rules: they apply to every key the solver chooses; a master is a key that is in
         # the masters of any core. `no_shape` holds the keys for which no bitting was found.
         self.shape = cfg.shape
-        self.shape_on = cfg.shape != SHAPE_OFF
-        self.masters = {m for core in cfg.cores for m in core["masters"]}
+        self.shape_on = not cfg.shape.is_off()
+        self.masters = cfg.master_keys
         self.no_shape = {}                    # key -> why no bitting follows the shape rules
 
         # Pinning: only for files that set it. The population of undecoded unit keys comes