From 31eff37abd82d62b2eafc87eff94d7c189ab04e1 Mon Sep 17 00:00:00 2001 From: Chelsea Kelly-Reif <3114598+ChelseaKR@users.noreply.github.com> Date: Fri, 2 Oct 2026 09:40:59 -0700 Subject: [PATCH 1/2] fix(eval): tuning-scope fingerprint ignores docstrings A docstring is an Expr(Constant(str)) statement, so ast.dump carried it and a prose-only edit to a tunable module moved the fingerprint exactly as a ranking change does. That demanded a Tunes-Against trailer no honest commit can carry. Every Python comparison the gate makes now drops the leading string of each module, class and function body before dumping. Bare strings elsewhere, f-strings and bytes literals stay in the tree. The premise (nothing in the package reads a docstring) is pinned by a test, and both directions are covered on every fingerprint path: docstring-only edits pass, same-size edits to the adjacent statement still fail. Closes #174 --- CHANGELOG.md | 14 +++ CONTRIBUTING.md | 6 +- docs/ROADMAP.md | 5 +- src/sprout/cli.py | 13 +-- src/sprout/eval/tuning_scope.py | 62 ++++++++++-- tests/test_tuning_scope.py | 169 ++++++++++++++++++++++++++++++++ 6 files changed, 254 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c30302..7d6eb61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,20 @@ fixes. Security entries reference the advisory (GHSA) per the portfolio release ## [Unreleased] +- **A docstring-only change to a tunable module can land without a `Tunes-Against:` trailer + (#174).** `tuning_scope._python_fingerprint` described itself as comment-insensitive, and it + was for `#` comments, which `ast` discards. A docstring is an `Expr(Constant(str))` statement, + though, so it was in the `ast.dump` and a one-word prose correction in + `src/sprout/providers/__init__.py` failed `sprout check-tuning-scope` exactly as a ranking + change would. There is no honest trailer for prose. Every Python comparison the gate makes + (the default one and the `answer.py`, `config.py` and provider-factory ones) now drops the + leading string of each module, class and function body first. A bare string anywhere else, + an f-string, and a bytes literal are still code. The exemption has the same standard as the + existing ones: nothing in the package reads `__doc__`, and + `test_package_never_reads_a_docstring` fails the day something does. Controls in both + directions run on every fingerprint path: a docstring-only edit passes, and a same-size + edit to the statement beside it still fails, with or without a docstring edit alongside. + - **The published site now loads Google Analytics 4, and a `/privacy/` page says what it records ([ADR 0023](docs/adr/0023-google-analytics-4-on-the-published-site.md)).** Per the owner's 2026-09-17 decision to run GA4 on every public site in the portfolio. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1e8b3ee..727235e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -144,7 +144,11 @@ failures** — never against results only visible from a private/local run. Mech executes — today just `Assistant.trace`, the `--debug` retrieval dump — because `sprout eval` replays `Assistant.answer` and never calls them, so nothing inside them can move an eval outcome; `tests/test_tuning_scope.py::test_eval_visible_modules_never_call_debug_only_methods` - fails the moment an eval-visible module calls one. Model literals, prompts, decoding parameters, + fails the moment an eval-visible module calls one. Python files are compared as syntax trees, so + `#` comments and docstrings (the leading string of a module, class or function body) are not + tuning either: nothing in the package reads `__doc__`, and + `tests/test_tuning_scope.py::test_package_never_reads_a_docstring` fails the day something does. + A bare string anywhere else is still code. Model literals, prompts, decoding parameters, real config values, retrieval/guard edits, module-level code in `answer.py`, every later lifecycle edit, and every unknown provider hunk remain fail-closed and require a real pre-existing case. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index f26c3d8..66fa2ea 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -318,8 +318,9 @@ a real URL.* - Done: tuning only against committed eval failures is mechanically enforced by `sprout check-tuning-scope`, a required CI job. Changes to retrieval, generation, guards, calibration, lexical logic, or config must cite a case already recorded in the committed - eval baseline via a `Tunes-Against:` commit trailer. Comment-only YAML and the exact named - operational lifecycle wrapper are excluded by semantic/AST comparison. The initial lifecycle + eval baseline via a `Tunes-Against:` commit trailer. Comment-only YAML, comment- and + docstring-only Python, and the exact named operational lifecycle wrapper are excluded by + semantic/AST comparison. The initial lifecycle module is pinned to one reviewed bootstrap digest; all later lifecycle hunks are gated. Authorization comes from the merge-base baseline, and adversarial tests keep model, prompt, decoding, real-config, retrieval/guard, lifecycle-output, and unknown provider edits fail-closed. diff --git a/src/sprout/cli.py b/src/sprout/cli.py index c4cb1f9..90089d4 100644 --- a/src/sprout/cli.py +++ b/src/sprout/cli.py @@ -629,12 +629,13 @@ def check_tuning_scope_cmd( """Fail if this change tunes retrieval/prompts/guards without citing an already-committed eval failure (ROADMAP Phase 3: tune only against committed failures, never the held-out set). - No-op when the diff does not semantically change the tunable surface. Comment-only YAML and - the exact named lifecycle wrapper around an otherwise-identical provider constructor are - compared mechanically and excluded. The initial lifecycle module is admitted once by its - reviewed digest; later lifecycle and unknown provider hunks fail closed. Otherwise the commit - range must carry a ``Tunes-Against: [, ...]`` trailer whose ids already - appear in the merge-base commit's ```` ``failing_examples``. + No-op when the diff does not semantically change the tunable surface. Comment-only YAML, + comment- and docstring-only Python, and the exact named lifecycle wrapper around an + otherwise-identical provider constructor are compared mechanically and excluded. The initial + lifecycle module is admitted once by its reviewed digest; later lifecycle and unknown provider + hunks fail closed. Otherwise the commit range must carry a + ``Tunes-Against: [, ...]`` trailer whose ids already appear in the + merge-base commit's ```` ``failing_examples``. """ from .eval.tuning_scope import TuningScopeError, check_tuning_scope diff --git a/src/sprout/eval/tuning_scope.py b/src/sprout/eval/tuning_scope.py index 4b7aa5d..5c92109 100644 --- a/src/sprout/eval/tuning_scope.py +++ b/src/sprout/eval/tuning_scope.py @@ -24,7 +24,8 @@ exist at the branch's merge base; every later lifecycle or unknown provider hunk fails closed. So are the ``Assistant`` methods an eval run never executes (``Assistant.trace``, the ``--debug`` dump): a gate on *eval outcomes* has nothing to say about code the eval never runs, and demanding -a ``Tunes-Against`` citation for it would only be satisfiable by writing a false one. +a ``Tunes-Against`` citation for it would only be satisfiable by writing a false one. Docstrings +are dropped from every Python comparison for the same reason: nothing in the package reads one. """ from __future__ import annotations @@ -117,6 +118,53 @@ def tunable_paths(changed: list[str]) -> list[str]: return sorted(p for p in changed if is_tunable_path(p)) +class _DropDocstrings(ast.NodeTransformer): + """Erase only docstrings: the leading ``str`` constant of a module, class or function body. + + ``ast`` already discards ``#`` comments, but a docstring is an ``Expr(Constant(str))`` + statement, so without this a one-word prose correction moves the fingerprint exactly as a + ranking constant does. No module under ``src/sprout/`` reads ``__doc__`` (pinned by + ``tests/test_tuning_scope.py::test_package_never_reads_a_docstring``), so a docstring + cannot reach a prompt, a ranking, a guard or a threshold. A bare string anywhere other than + the first statement of a body, an f-string, and a bytes literal are not docstrings and stay + in the tree. + """ + + @staticmethod + def _strip(body: list[ast.stmt]) -> list[ast.stmt]: + if ( + body + and isinstance(body[0], ast.Expr) + and isinstance(body[0].value, ast.Constant) + and isinstance(body[0].value.value, str) + ): + return body[1:] or [ast.Pass()] + return body + + def visit_Module(self, node: ast.Module) -> ast.AST: + node.body = self._strip(node.body) + return self.generic_visit(node) + + def visit_ClassDef(self, node: ast.ClassDef) -> ast.AST: + node.body = self._strip(node.body) + return self.generic_visit(node) + + def visit_FunctionDef(self, node: ast.FunctionDef) -> ast.AST: + node.body = self._strip(node.body) + return self.generic_visit(node) + + def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> ast.AST: + node.body = self._strip(node.body) + return self.generic_visit(node) + + +def _parse_without_docstrings(source: str, *, type_comments: bool = False) -> ast.Module: + """Parse ``source`` and drop its docstrings; every fingerprint below starts here.""" + tree = ast.parse(source, type_comments=type_comments) + _DropDocstrings().visit(tree) # in place: visit_Module returns the same node + return tree + + class _LifecycleWrapperNormalizer(ast.NodeTransformer): """Erase only the mechanically constrained operational wrapper seam. @@ -164,7 +212,7 @@ def visit_Call(self, node: ast.Call) -> ast.AST: def _provider_factory_fingerprint(source: str) -> str: - tree = ast.parse(source) + tree = _parse_without_docstrings(source) normalized = _LifecycleWrapperNormalizer().visit(tree) ast.fix_missing_locations(normalized) return ast.dump(normalized, include_attributes=False) @@ -221,21 +269,23 @@ def visit_ClassDef(self, node: ast.ClassDef) -> ast.AST: def _answer_without_eval_invisible_methods(source: str) -> str: - tree = _DropEvalInvisibleAssistantMethods().visit(ast.parse(source)) + tree = _DropEvalInvisibleAssistantMethods().visit(_parse_without_docstrings(source)) ast.fix_missing_locations(tree) return ast.dump(tree, include_attributes=False) def _config_without_eval_invisible_classes(source: str) -> str: - tree = _DropEvalInvisibleConfigClasses().visit(ast.parse(source)) + tree = _DropEvalInvisibleConfigClasses().visit(_parse_without_docstrings(source)) ast.fix_missing_locations(tree) return ast.dump(tree, include_attributes=False) def _python_fingerprint(source: str) -> str: - """Return a formatting- and comment-insensitive Python syntax fingerprint.""" + """Return a formatting-, comment- and docstring-insensitive Python syntax fingerprint.""" - return ast.dump(ast.parse(source, type_comments=True), include_attributes=False) + tree = _parse_without_docstrings(source, type_comments=True) + ast.fix_missing_locations(tree) + return ast.dump(tree, include_attributes=False) def _source_at(ref: str, path: str, *, cwd: str | Path) -> str | None: diff --git a/tests/test_tuning_scope.py b/tests/test_tuning_scope.py index 9173481..b8efc73 100644 --- a/tests/test_tuning_scope.py +++ b/tests/test_tuning_scope.py @@ -713,3 +713,172 @@ def test_commit_messages_reads_full_body(repo: Path) -> None: def test_bad_ref_raises_tuning_scope_error(repo: Path) -> None: with pytest.raises(TuningScopeError): check_tuning_scope(base_ref="does-not-exist", repo_root=repo) + + +# --- docstrings are prose, not tunable surface (#174) ------------------------------------ +# +# A docstring is an `Expr(Constant(str))` statement, so `ast.dump` carries it and editing one +# used to move the fingerprint exactly as editing a ranking constant does. Nothing in the +# package reads a docstring (pinned below), so it cannot move an eval outcome: the same +# standard the config-class and debug-method exemptions meet. + +# Every path with its own fingerprint function, plus one that takes the default +# `_python_fingerprint`, so the exemption is proven on each comparison the gate makes. +_DOCSTRING_PATHS = ( + "src/sprout/guards.py", + "src/sprout/answer.py", + "src/sprout/config.py", + "src/sprout/providers/__init__.py", +) + + +def _documented_module(doc: str, floor: str = "0.30") -> str: + # Module, class, method, async method, and a function whose body is only a docstring. + return ( + f'"""Module {doc}."""\n' + "\n" + f"FLOOR = {floor}\n" + "\n" + "\n" + "class Ranker:\n" + f' """Class {doc}."""\n' + "\n" + " def rank(self, score):\n" + f' """Method {doc}."""\n' + " return score >= FLOOR\n" + "\n" + " async def arank(self, score):\n" + f' """Async method {doc}."""\n' + " return score >= FLOOR\n" + "\n" + "\n" + "def only_a_docstring():\n" + f' """Function {doc}."""\n' + ) + + +def _seed_main(repo: Path, rel: str, text: str) -> None: + """Land ``text`` at ``rel`` on main and start ``work`` from there, so the next commit on + ``work`` is compared against exactly this content.""" + _git(["checkout", "-q", "main"], repo) + (repo / rel).write_text(text, encoding="utf-8") + _git(["commit", "-q", "-am", "chore: seed documented module"], repo) + _git(["checkout", "-q", "work"], repo) + _git(["reset", "-q", "--hard", "main"], repo) + + +@pytest.mark.parametrize("rel", _DOCSTRING_PATHS) +def test_docstring_only_change_passes_without_trailer(repo: Path, rel: str) -> None: + _seed_main(repo, rel, _documented_module("names the wrong package")) + (repo / rel).write_text(_documented_module("names the package this project publishes")) + _git(["commit", "-q", "-am", "docs: correct the install name in a docstring"], repo) + assert check_tuning_scope(base_ref="main", repo_root=repo) == [] + + +@pytest.mark.parametrize("rel", _DOCSTRING_PATHS) +def test_adjacent_statement_edit_still_requires_real_case(repo: Path, rel: str) -> None: + # The control: the same module, the same size of edit, one line away from the + # docstrings, and it is a statement. It must still fail with and without a docstring + # edit riding along. + _seed_main(repo, rel, _documented_module("v1")) + (repo / rel).write_text(_documented_module("v1", floor="0.35")) + _git(["commit", "-q", "-am", "fix: move the floor"], repo) + issues = check_tuning_scope(base_ref="main", repo_root=repo) + assert len(issues) == 1 + assert rel in issues[0] + + (repo / rel).write_text(_documented_module("v2", floor="0.35")) + _git(["commit", "-q", "-am", "docs: and reword the docstrings"], repo) + issues = check_tuning_scope(base_ref="main", repo_root=repo) + assert len(issues) == 1 + assert rel in issues[0] + + +def test_bare_string_outside_docstring_position_is_still_code(repo: Path) -> None: + # Only the *leading* string of a body is a docstring. A bare string anywhere else is a + # statement somebody wrote for a reason, and editing it is not exempt. + def source(note: str) -> str: + return ( + '"""Guards."""\n' + "\n" + "def check(text):\n" + ' """Return True when text is allowed."""\n' + f' "{note}"\n' + " return text not in DENIED\n" + ) + + _seed_main(repo, "src/sprout/guards.py", source("note v1")) + (repo / "src" / "sprout" / "guards.py").write_text(source("note v2"), encoding="utf-8") + _git(["commit", "-q", "-am", "chore(guards): edit a bare string"], repo) + issues = check_tuning_scope(base_ref="main", repo_root=repo) + assert len(issues) == 1 + assert "src/sprout/guards.py" in issues[0] + + +@pytest.mark.parametrize( + ("base", "head"), + [ + # A docstring moved below a statement is no longer a docstring. + ('"""Doc."""\nFLOOR = 0.30\n', 'FLOOR = 0.30\n"""Doc."""\n'), + # Same-length value edit beside an untouched docstring. + ('"""Doc."""\nFLOOR = 0.30\n', '"""Doc."""\nFLOOR = 0.35\n'), + # An f-string and a bytes literal in leading position are not docstrings. + ('f"{FLOOR}"\nFLOOR = 0.30\n', 'f"{FLOOR}!"\nFLOOR = 0.30\n'), + ("b'x'\nFLOOR = 0.30\n", "b'y'\nFLOOR = 0.30\n"), + # Removing a function body down to its docstring changes behavior. + ('def f():\n """Doc."""\n return 1\n', 'def f():\n """Doc."""\n'), + ], +) +def test_python_fingerprint_still_moves_for_code(base: str, head: str) -> None: + from sprout.eval.tuning_scope import _python_fingerprint + + assert _python_fingerprint(base) != _python_fingerprint(head) + + +@pytest.mark.parametrize( + ("base", "head"), + [ + ('"""Old."""\nFLOOR = 0.30\n', '"""New, and longer."""\nFLOOR = 0.30\n'), + ("FLOOR = 0.30\n", '"""Added."""\nFLOOR = 0.30\n'), + ("class C:\n x = 1\n", 'class C:\n """Added."""\n x = 1\n'), + ('def f():\n """Old."""\n', 'def f():\n """New."""\n'), + ("def f():\n pass\n", 'def f():\n """Only a docstring now."""\n'), + ], +) +def test_python_fingerprint_ignores_docstrings(base: str, head: str) -> None: + from sprout.eval.tuning_scope import _python_fingerprint + + assert _python_fingerprint(base) == _python_fingerprint(head) + + +def test_package_never_reads_a_docstring() -> None: + # The invariant that keeps the docstring exemption sound: no module in the package + # reads one, so no docstring can reach a prompt, a ranking, a guard or a threshold. The + # day one does, this fails and the exemption must be narrowed before the change lands, + # instead of quietly covering a prompt. Pydantic's attribute-docstring and JSON-schema + # paths are named too, because both turn a class docstring into data. (Typer reads the + # docstrings of the CLI commands in `cli.py` as `--help` text; `cli.py` is not tunable + # surface and an eval run never calls it.) + import ast as _ast + + readers = {"__doc__", "getdoc", "get_docstring", "model_json_schema"} + modules = sorted(Path("src/sprout").rglob("*.py")) + assert len(modules) >= 20, "the scan stopped finding the package" + offenders = [] + for module in modules: + tree = _ast.parse(module.read_text(encoding="utf-8")) + for node in _ast.walk(tree): + name = ( + node.attr + if isinstance(node, _ast.Attribute) + else node.id + if isinstance(node, _ast.Name) + else node.value + if isinstance(node, _ast.Constant) and isinstance(node.value, str) + else node.arg + if isinstance(node, _ast.keyword) + else None + ) + if name in readers or name == "use_attribute_docstrings": + offenders.append(f"{module}:{getattr(node, 'lineno', '?')}") + assert not offenders, f"package module(s) read a docstring: {offenders}" From 16e2f5364c6a2c01274d07fda189070708f8f8d8 Mon Sep 17 00:00:00 2001 From: Chelsea Kelly-Reif <3114598+ChelseaKR@users.noreply.github.com> Date: Fri, 2 Oct 2026 09:41:45 -0700 Subject: [PATCH 2/2] docs(providers): name the published distribution in the factory docstring The module docstring told readers that `pip install sprout` runs end to end with no extras. That installs an unrelated library. Before the previous commit the tuning-scope gate counted this one-word prose fix as tuning, so it was held back as the single KNOWN_GAP entry in test_install_instructions.py. It is corrected now, and the exemption and the test that pinned it are removed as their own comment said they should be. Refs #174 --- CHANGELOG.md | 3 ++ src/sprout/providers/__init__.py | 2 +- tests/test_install_instructions.py | 48 ++---------------------------- 3 files changed, 6 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d6eb61..d798fd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ fixes. Security entries reference the advisory (GHSA) per the portfolio release `test_package_never_reads_a_docstring` fails the day something does. Controls in both directions run on every fingerprint path: a docstring-only edit passes, and a same-size edit to the statement beside it still fails, with or without a docstring edit alongside. + The motivating docstring, which told readers to `pip install sprout` (an unrelated + library), is corrected. Its `KNOWN_GAP` entry and `test_the_known_gap_is_still_a_gap` in + `tests/test_install_instructions.py` are removed with it, as that entry said they should be. - **The published site now loads Google Analytics 4, and a `/privacy/` page says what it records ([ADR 0023](docs/adr/0023-google-analytics-4-on-the-published-site.md)).** Per the diff --git a/src/sprout/providers/__init__.py b/src/sprout/providers/__init__.py index 6c53f6c..1a64221 100644 --- a/src/sprout/providers/__init__.py +++ b/src/sprout/providers/__init__.py @@ -1,7 +1,7 @@ """Provider factory: config strings -> concrete embedder/generator, lazily imported. Lazy imports keep the offline default free of heavy dependencies — ``boto3`` is only -imported when ``provider: bedrock`` is actually selected, so ``pip install sprout`` with +imported when ``provider: bedrock`` is actually selected, so ``pip install sprout-plantcare`` with no extras still runs end to end. ``build_entailment_verifier`` follows the identical pattern for the ``sprout[nli]`` extra (EXP-04): ``onnxruntime``/``tokenizers`` are only imported when ``generation.support_verifier: nli`` is actually configured. diff --git a/tests/test_install_instructions.py b/tests/test_install_instructions.py index d9ccefb..712f15c 100644 --- a/tests/test_install_instructions.py +++ b/tests/test_install_instructions.py @@ -67,21 +67,6 @@ #: read all of it, so the population is asserted rather than assumed. MIN_CODE_FILES = 150 -#: The one occurrence this change could not correct, named rather than quietly skipped. -#: -#: `src/sprout/providers/` is `TUNABLE_SURFACE`, and `tuning_scope._python_fingerprint` -#: builds its comparison from `ast.dump`, where a **docstring is a statement**. So a -#: correction confined to a module docstring — text nothing in `src/` reads, measured: -#: `__doc__` appears nowhere in the package — is indistinguishable to that gate from a -#: change to retrieval ranking, and it demands a `Tunes-Against:` trailer citing a -#: committed eval failure. There is no honest trailer for a comment, and writing a false -#: one to get past a gate is worse than the wrong sentence it would fix. -#: -#: This is an exemption, so it is self-limiting: `test_the_known_gap_is_still_a_gap` -#: fails the day the file is corrected or the gate learns to ignore docstrings, and the -#: entry has to be deleted then. It cannot outlive its reason quietly. -KNOWN_GAP = frozenset({"src/sprout/providers/__init__.py"}) - def _git(*args: str) -> str | None: executable = shutil.which("git") @@ -144,8 +129,8 @@ def test_no_install_command_in_code_names_a_package_this_project_does_not_publis Ten occurrences in seven files, two of them exception messages raised at runtime telling a user the exact command to run to get an optional extra — the loudest possible place to name the wrong package, and the only place in the repository - nobody re-reads on the way past. Nine are corrected; the tenth is in `KNOWN_GAP` - above with its reason and its expiry. + nobody re-reads on the way past. All ten are corrected; the last waited on #174, a + tuning-scope gate that counted a docstring as code. """ distribution = declared_distribution() files = _tracked_code_files() @@ -156,8 +141,6 @@ def test_no_install_command_in_code_names_a_package_this_project_does_not_publis ) problems: list[str] = [] for path in files: - if str(path.relative_to(ROOT)) in KNOWN_GAP: - continue text = path.read_text(encoding="utf-8") for line_number, line in enumerate(text.splitlines(), 1): for target in wrongly_named_installs(line, distribution): @@ -170,33 +153,6 @@ def test_no_install_command_in_code_names_a_package_this_project_does_not_publis ) -def test_the_known_gap_is_still_a_gap() -> None: - """An exemption that has stopped exempting anything reads as coverage. - - Every entry above has to still contain the thing it is excused for. The day the - docstring is corrected — or the day `tuning_scope` stops treating one as a behavior - change — this fails and the entry goes, rather than sitting in the tuple looking like - a considered decision about a file nobody has looked at in a year. - - It also pins the count, because an exemption list is the one list where growth is the - signal: a second entry means somebody widened an excuse instead of fixing a file. - """ - distribution = declared_distribution() - assert len(KNOWN_GAP) == 1, ( - f"the exemption list has grown to {sorted(KNOWN_GAP)}. Adding an entry here excuses a " - "file from naming the right package; it is not a place to put a file that is merely " - "inconvenient to fix." - ) - for name in sorted(KNOWN_GAP): - path = ROOT / name - assert path.is_file(), f"{name} is exempted here and is not in the tree" - assert wrongly_named_installs(path.read_text(encoding="utf-8"), distribution), ( - f"{name} is exempted from the scan above and no longer contains an install " - "command naming the wrong package. Delete the entry — an exemption that exempts " - "nothing is an exemption nobody can see the cost of." - ) - - def test_the_package_derives_its_version_from_the_distribution_the_manifest_declares() -> None: """The rename's silent failure mode, made mechanical instead of remembered.