DEV Community

Quinn Sun
Quinn Sun

Posted on

The Pairing Session Kept the Invariant Comment After the Clarity Pass Went Green

A Thursday pairing session stalled on a ninety-line config loader. An assistant had offered a clarity pass: shorter names, fewer comments, and a local test run that stayed green. The pairing partner wanted the patch merged before lunch. The senior did not open the merge button.

The scene is a reconstructed pairing session, assembled so the questions, dead ends, and kept decision can be replayed. It is not a report of a named outage, a customer, or a measured benchmark. The files below are an unexecuted example. Copy them into a scratch directory and run the commands before treating the checker as team policy.

The questions came before the diff

The loader still carried three comments marked INVARIANT:. One required a missing region to fail closed. One allowed an empty channels list. One rejected timeout_ms above 30000 even when the rest of the file parsed. The clarity pass deleted all three and renamed timeout_ms to timeout.

The senior asked four questions and wrote the answers on a shared note:

  1. Which of those sentences is still true in the new function?
  2. Which test fails if that sentence becomes false?
  3. Did the draft preserve behavior, or only preserve a green suite?
  4. Where does the next reader learn the fail-closed rule if the comment is gone?

The partner answered the third question with the test log. That answer ended the first path.

Dead end one: green tests that never encoded the comment

The existing tests built a complete manifest and checked a happy-path object. They never omitted region. They never passed a timeout above the commented ceiling. A deleted comment and a renamed field could both land while the suite stayed green.

The suite had never subscribed to the sentences the comments were holding. A longer test log did not repair that gap. The partner's next move was to ask the assistant for a pull-request summary of the old comments. The senior rejected the summary as evidence. A chat paragraph is not a regression check. The next edit will not fail because a transcript once mentioned region.

A third attempt added one test for the missing region and stopped. One sentence became executable. The other two stayed as folklore. The session parked the merge there, with the clarity pass still looking tidy in the diff viewer.

Dead end two: a rename sold as clarity

The field rename looked harmless in isolation. Callers in the same repository still passed timeout_ms. The draft either dropped those keys silently or required a second, broader edit that the clarity pass did not include.

The senior treated the rename as a compatibility question, not a style question. A shorter name is not clearer if every manifest in staging still uses the old key. The session reverted the rename and left a follow-up ticket. Clarity, in that note, meant a reader can predict the failure mode. It did not mean fewer lines in the diff.

The decision that stayed on the branch

Ordinary comments can go. A comment that states a fail-closed or compatibility rule stays until a test names that rule, or until a reviewed commit retires the rule and the test together.

The kept artifact is a small checker, check_invariants.py, plus a fixture module and three tests. The checker does not execute the loader. It scans non-test Python files for INVARIANT: slugs and demands the same slug in a test file. An asymmetric clarity pass, comment deleted and no paired test, exits non-zero.

A reviewed removal of both the comment and the test still passes, which is intentional. That case is a behavior change and belongs in the pull-request note, not in a regex. The session refused to let a formatting label hide that choice.

Fixture, tests, and checker

Save these files in an empty directory. The checker uses only the Python standard library. pytest is used for the behavior tests. Nothing here calls a network service.

loader.py:

def load_manifest(doc):
    # INVARIANT: missing-region-fails-closed
    if "region" not in doc or not doc["region"]:
        raise ValueError("region is required")
    # INVARIANT: empty-channels-are-valid
    channels = doc.get("channels", [])
    if not isinstance(channels, list):
        raise ValueError("channels must be a list")
    # INVARIANT: timeout-above-30s-rejected
    timeout_ms = doc.get("timeout_ms", 1000)
    if not isinstance(timeout_ms, int) or timeout_ms <= 0 or timeout_ms > 30000:
        raise ValueError("timeout_ms must be in 1..30000")
    return {
        "region": doc["region"],
        "channels": channels,
        "timeout_ms": timeout_ms,
    }
Enter fullscreen mode Exit fullscreen mode

test_loader.py:

import pytest
from loader import load_manifest

def test_missing_region_fails_closed():
    """Covers INVARIANT: missing-region-fails-closed."""
    with pytest.raises(ValueError):
        load_manifest({"channels": ["alerts"]})

def test_empty_channels_are_valid():
    """Covers INVARIANT: empty-channels-are-valid."""
    got = load_manifest({"region": "us-east-1", "channels": [], "timeout_ms": 500})
    assert got["channels"] == []

def test_timeout_above_30s_rejected():
    """Covers INVARIANT: timeout-above-30s-rejected."""
    with pytest.raises(ValueError):
        load_manifest({"region": "us-east-1", "timeout_ms": 30001})
Enter fullscreen mode Exit fullscreen mode

check_invariants.py:

import pathlib
import re
import sys

MARK = re.compile(r"INVARIANT:\s*([a-z0-9][a-z0-9-]*)")

def slugs_in(path):
    return set(MARK.findall(path.read_text(encoding="utf-8")))

def main():
    root = pathlib.Path(sys.argv[1] if len(sys.argv) > 1 else ".")
    sources = [
        p for p in root.glob("*.py")
        if not p.name.startswith("test_") and p.name != "check_invariants.py"
    ]
    tests = list(root.glob("test_*.py"))
    required = set()
    for src in sources:
        required |= slugs_in(src)
    covered = set()
    for test in tests:
        covered |= slugs_in(test)
    missing = sorted(required - covered)
    if missing:
        print("invariant comments without a paired test slug:")
        for slug in missing:
            print("  - " + slug)
        return 1
    print("paired %d invariant comment(s)" % len(required))
    return 0

if __name__ == "__main__":
    raise SystemExit(main())
Enter fullscreen mode Exit fullscreen mode

The whiteboard commands were for a POSIX shell:

python3 -m venv .venv
. .venv/bin/activate
python -m pip install pytest
python check_invariants.py .
python -m pytest -q
Enter fullscreen mode Exit fullscreen mode

Failure drill

Delete only the comment line that contains timeout-above-30s-rejected. Leave the raise in load_manifest. Delete test_timeout_above_30s_rejected as well. Run python check_invariants.py . again. The process should exit 1 and print that slug.

Restore the test docstring slug and the checker should exit 0, even if a later edit breaks the assertion inside the test. That second result is a limitation, not a proof. The regex never calls load_manifest. Slugs must stay lowercase with hyphens, because the pattern ignores other shapes. A comment written as Invariant: Missing Region will not be seen.

What the pairing note recorded

The kept note was short enough to paste above the diff:

  • Slugs kept: missing-region-fails-closed, empty-channels-are-valid, timeout-above-30s-rejected.
  • Rename of timeout_ms: reverted. Not a clarity change.
  • Assistant draft: exploration only. Not cited as coverage.
  • Checker: blocks unmatched INVARIANT: slugs. Does not prove the invariant.

The partner also wrote who ran the commands and on which commit. A note without a commit id was treated as a conversation, not a record.

Where a free exploration pass fits

Disclosure: This article was prepared as part of MonkeyCode's product outreach.

The exploration pass is the part a free coding model can actually help. The partner used a free model access path, and a free server option, to request a clarity rewrite and then to request tests that named the three slugs. Both drafts were disposable. The merge decision stayed on the local checker, the pytest run, and the pairing note.

That split is the only product claim this write-up relies on: free model access for a throwaway draft, and a free server option so the scratch run is not tied to one laptop. Numeric quotas, hardware sizes, model names, and how long either option lasts are omitted on purpose. They were not attached as a primary source, and they go stale. Read the current project documentation before anyone plans a sprint around them.

A usable split from the session:

  • Generate the candidate rewrite and the candidate tests on the free server.
  • Discard any draft that renames a public key without a caller migration.
  • Run check_invariants.py and pytest on a machine the reviewers control.
  • Write the slug list into the pull request. Do not paste a chat transcript as the proof.

Decision table

Signal Treat as Next action
Comment is a citation, a joke, or a stale TODO Style Delete freely
Comment states fail-closed or compatibility behavior Obligation Keep until a test slug matches
Suite is green and the obligation comment is gone Incomplete Block the clarity pass
Comment and its test leave in one reviewed commit Behavior change Allow, with a PR note
Summary exists only in the assistant chat Non-evidence Do not cite it as coverage
Field rename has no caller migration Compatibility risk Revert in this patch
Free-server draft matches the fixture Candidate Still run the local checker

Who should skip this

Teams with no marked invariants, and no plan to maintain the slug convention, will only gain another warning to silence. Generated clients that overwrite comments on every release should bind tests to the schema instead. A regex over a generated header will flap.

This checker is not a security review. It does not walk a call graph, does not prove the exception is raised on every path, and does not inspect network or file side effects. A green checker plus a green pytest run still needs a human to read the diff. The example also assumes a single directory of *.py files. A monorepo layout needs a real file list, which this script does not discover.

Do not upload secrets, production manifests, or customer payloads to a hosted assistant because a server option is free. The fixture here is synthetic for that reason. A free allowance, whatever the current number is, does not change the data-handling rule.

What the session refused to conclude

The partner wanted a rule that clear code means fewer comments. The senior would not sign that. The comments that mattered were the ones carrying obligations the suite had never asserted. Removing them made the file look cleaner and made the next regression quieter.

The decision that stayed was small. An INVARIANT: line is a debt the next patch pays with a paired test, or retires on purpose. A free model can draft that test. It does not get to witness that the test is enough.

Readers who already separate exploration from the merge gate can use MonkeyCode's free model access and free server option for the throwaway draft, then keep the decision on the checker in this article. Confirm the live terms on the project page before relying on them.

Top comments (0)