From 3c79b72e190098dacec83bb8b2f939e957a24c2a Mon Sep 17 00:00:00 2001 From: j-kemble Date: Sun, 16 Aug 2026 19:59:55 -0500 Subject: [PATCH 1/3] =?UTF-8?q?fix:=20resolve=20PR=20#3=20review=20finding?= =?UTF-8?q?s=20=E2=80=94=20constant=20sync,=20strength-bar=20semantic,=20s?= =?UTF-8?q?tale=20comment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - fix(constants): USERNAME_ALPHANUMERIC lowercase-only (was mixed-case) - fix(tui_render): MAX_USERNAME_LENGTH for username strength bar (was MAX_PASSWORD_CHARS, which happens to equal 64 but is semantically wrong) - fix(tui): stale comment says tui_render 'aliases' helpers when it now redefines them locally - test: add test_constants_sync.py as a guard against future constant drift Gates: pytest 490/490, bandit 0, mypy 0 (verified at fe8e532 baseline). --- generate_it/constants.py | 2 +- generate_it/tui.py | 6 +- generate_it/tui_render.py | 2 +- tests/test_constants_sync.py | 113 +++++++++++++++++++++++++++++++++++ 4 files changed, 118 insertions(+), 5 deletions(-) create mode 100644 tests/test_constants_sync.py diff --git a/generate_it/constants.py b/generate_it/constants.py index 0eb6e67..295bbfc 100644 --- a/generate_it/constants.py +++ b/generate_it/constants.py @@ -67,7 +67,7 @@ SPECIAL_CHARACTERS = "!@#$%^&*()-_=+[]{};:,.?/" # Username-related character sets. -USERNAME_ALPHANUMERIC = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" +USERNAME_ALPHANUMERIC = "abcdefghijklmnopqrstuvwxyz0123456789" USERNAME_SEPARATORS = frozenset(["_", "-"]) # Username styles diff --git a/generate_it/tui.py b/generate_it/tui.py index ba28539..4f1adf5 100644 --- a/generate_it/tui.py +++ b/generate_it/tui.py @@ -2063,9 +2063,9 @@ def _gen_pwd(): return 0 -# Imported last to avoid a circular import: tui_render aliases a few -# state-label helpers (e.g. _selected_category_count) that are defined -# above in this module. +# Imported last to avoid a circular import: tui_render now redefines a few +# state-label helpers (e.g. _selected_category_count) locally rather +# than importing them from this module. from . import tui_render as R from . import tui_flow diff --git a/generate_it/tui_render.py b/generate_it/tui_render.py index 3411f1b..aa91aef 100644 --- a/generate_it/tui_render.py +++ b/generate_it/tui_render.py @@ -856,7 +856,7 @@ def _render_info_box( + math.log2(len(generator.PASSPHRASE_SPECIALS)) ) else: - max_bits = float(generator.MAX_PASSWORD_CHARS) * math.log2( + max_bits = float(generator.MAX_USERNAME_LENGTH) * math.log2( len(generator.USERNAME_ALPHANUMERIC) ) bar = _bar(min(bits, max_bits), max_bits, bar_w) diff --git a/tests/test_constants_sync.py b/tests/test_constants_sync.py new file mode 100644 index 0000000..849b78d --- /dev/null +++ b/tests/test_constants_sync.py @@ -0,0 +1,113 @@ +"""Test that every constant duplicated in ``constants.py`` matches its +counterpart in ``generator.py``. + +The duplication between ``generate_it/constants.py`` and +``generate_it/generator.py`` is a known architectural debt — the former was +extracted as a central registry after the latter already defined many module- +level constants. This test guards against silent drift (see +``review-REJECTED-consolidated-remediation-20260816-2305.md`` §3, which caught +``USERNAME_ALPHANUMERIC`` having diverged). + +If this test fails, the two modules are out of sync. Fix the source of truth +(first decide which one it is, then update the other) and add the correction +to this test so future drift is caught. +""" + +from __future__ import annotations + +import string + +from generate_it import constants +from generate_it import generator + + +def _as_str(value: object) -> str: + """Normalise a constant to a string for comparison. + + Both modules may express string constants as plain ``str``, via + ``string.ascii_letters``, or as ``frozenset``. Normalise everything + to a plain ``str`` for unambiguous comparison. + """ + if isinstance(value, frozenset): + return str(sorted(value)) + return str(value) + + +# --- Numeric boundaries --------------------------------------------------- + +SHARED_INTS: dict[str, int] = { + "MIN_PASSWORD_CHARS": 8, + "MAX_PASSWORD_CHARS": 64, + "MIN_PASSPHRASE_WORDS": 3, + "MAX_PASSPHRASE_WORDS": 10, + "MIN_USERNAME_LENGTH": 3, + "MAX_USERNAME_LENGTH": 64, + "MIN_USERNAME_WORDS": 1, + "MAX_USERNAME_WORDS": 3, + "_MIN_PASSPHRASE_ENTROPY_BITS": 50, +} + + +class TestConstantsSync: + """Verify every shared constant matches between ``constants`` and ``generator``.""" + + # --- Integer/bool constants ----------------------------------------- + + def test_min_password_chars(self) -> None: + assert constants.MIN_PASSWORD_CHARS == generator.MIN_PASSWORD_CHARS + + def test_max_password_chars(self) -> None: + assert constants.MAX_PASSWORD_CHARS == generator.MAX_PASSWORD_CHARS + + def test_min_passphrase_words(self) -> None: + assert constants.MIN_PASSPHRASE_WORDS == generator.MIN_PASSPHRASE_WORDS + + def test_max_passphrase_words(self) -> None: + assert constants.MAX_PASSPHRASE_WORDS == generator.MAX_PASSPHRASE_WORDS + + def test_min_username_length(self) -> None: + assert constants.MIN_USERNAME_LENGTH == generator.MIN_USERNAME_LENGTH + + def test_max_username_length(self) -> None: + assert constants.MAX_USERNAME_LENGTH == generator.MAX_USERNAME_LENGTH + + def test_min_username_words(self) -> None: + assert constants.MIN_USERNAME_WORDS == generator.MIN_USERNAME_WORDS + + def test_max_username_words(self) -> None: + assert constants.MAX_USERNAME_WORDS == generator.MAX_USERNAME_WORDS + + def test_min_passphrase_entropy_bits(self) -> None: + assert constants._MIN_PASSPHRASE_ENTROPY_BITS == generator._MIN_PASSPHRASE_ENTROPY_BITS + + # --- Character-set string constants ---------------------------------- + + def test_letters(self) -> None: + assert _as_str(constants.LETTERS) == _as_str(generator.LETTERS) + + def test_numbers(self) -> None: + assert _as_str(constants.NUMBERS) == _as_str(generator.NUMBERS) + + def test_special_characters(self) -> None: + assert _as_str(constants.SPECIAL_CHARACTERS) == _as_str(generator.SPECIAL_CHARACTERS) + + def test_username_alphanumeric(self) -> None: + """This was the constant that diverged — PR #3 review caught it.""" + assert ( + _as_str(constants.USERNAME_ALPHANUMERIC) + == _as_str(generator.USERNAME_ALPHANUMERIC) + ) + # The canonical value — both must be lowercase-only + expected = string.ascii_lowercase + string.digits + assert _as_str(constants.USERNAME_ALPHANUMERIC) == expected + + def test_username_separators(self) -> None: + assert _as_str(constants.USERNAME_SEPARATORS) == _as_str(generator.USERNAME_SEPARATORS) + + def test_passphrase_specials(self) -> None: + assert _as_str(constants.PASSPHRASE_SPECIALS) == _as_str(generator.PASSPHRASE_SPECIALS) + + # --- Constant collections (lists / frozensets) ----------------------- + + def test_default_wordlist(self) -> None: + assert constants.DEFAULT_WORDLIST == generator.DEFAULT_WORDLIST \ No newline at end of file From cd2e316a8324d8d181acd39a000d42602c1e09ef Mon Sep 17 00:00:00 2001 From: j-kemble Date: Sun, 16 Aug 2026 21:10:45 -0500 Subject: [PATCH 2/3] fix(ui): drop entropy/strength bar from username mode (usernames are not entropy-regulated) - Guard the entropy computation, strength label, and strength bar with state.mode != "username" in _render_info_box() - Move bits/label/kind/kind_attr computation inside the guard so _estimate_entropy_bits is never called with username mode - Remove the else (username) branch from the max_bits computation - Remove dead SHARED_INTS dict from test_constants_sync.py - Add missing trailing newline to test_constants_sync.py --- generate_it/tui_render.py | 61 +++++++++++++++++------------------- tests/test_constants_sync.py | 17 +--------- 2 files changed, 30 insertions(+), 48 deletions(-) diff --git a/generate_it/tui_render.py b/generate_it/tui_render.py index aa91aef..6402a39 100644 --- a/generate_it/tui_render.py +++ b/generate_it/tui_render.py @@ -772,16 +772,6 @@ def _render_info_box( inner_w = max(0, w - 4) row = y + 1 - bits = _estimate_entropy_bits(state, wordlist_size) - label, kind = _strength_label(bits) - - if kind == "bad": - kind_attr = theme.bad - elif kind == "warn": - kind_attr = theme.warn - else: - kind_attr = theme.ok - mode_str = ( "characters" if state.mode == "chars" @@ -838,26 +828,33 @@ def _render_info_box( ) row += 1 - # Strength bar - _addstr_safe(stdscr, row, x + 2, f"Entropy: ~{bits:0.1f} bits"[:inner_w], theme.dim) - row += 1 + if state.mode != "username": + bits = _estimate_entropy_bits(state, wordlist_size) + label, kind = _strength_label(bits) - prefix = "Strength: [" - suffix = f"] {label}" - bar_w = max(0, inner_w - len(prefix) - len(suffix)) - if state.mode == "chars": - max_bits = float(generator.MAX_PASSWORD_CHARS) * math.log2( - len(generator.LETTERS) + len(generator.NUMBERS) + len(generator.SPECIAL_CHARACTERS) - ) - elif state.mode == "words": - max_bits = ( - float(generator.MAX_PASSPHRASE_WORDS) * math.log2(max(2, wordlist_size)) - + 3.0 * math.log2(10.0) - + math.log2(len(generator.PASSPHRASE_SPECIALS)) - ) - else: - max_bits = float(generator.MAX_USERNAME_LENGTH) * math.log2( - len(generator.USERNAME_ALPHANUMERIC) - ) - bar = _bar(min(bits, max_bits), max_bits, bar_w) - _addstr_safe(stdscr, row, x + 2, f"{prefix}{bar}{suffix}"[:inner_w], kind_attr) + if kind == "bad": + kind_attr = theme.bad + elif kind == "warn": + kind_attr = theme.warn + else: + kind_attr = theme.ok + + # Strength bar + _addstr_safe(stdscr, row, x + 2, f"Entropy: ~{bits:0.1f} bits"[:inner_w], theme.dim) + row += 1 + + prefix = "Strength: [" + suffix = f"] {label}" + bar_w = max(0, inner_w - len(prefix) - len(suffix)) + if state.mode == "chars": + max_bits = float(generator.MAX_PASSWORD_CHARS) * math.log2( + len(generator.LETTERS) + len(generator.NUMBERS) + len(generator.SPECIAL_CHARACTERS) + ) + elif state.mode == "words": + max_bits = ( + float(generator.MAX_PASSPHRASE_WORDS) * math.log2(max(2, wordlist_size)) + + 3.0 * math.log2(10.0) + + math.log2(len(generator.PASSPHRASE_SPECIALS)) + ) + bar = _bar(min(bits, max_bits), max_bits, bar_w) + _addstr_safe(stdscr, row, x + 2, f"{prefix}{bar}{suffix}"[:inner_w], kind_attr) diff --git a/tests/test_constants_sync.py b/tests/test_constants_sync.py index 849b78d..72ac4a1 100644 --- a/tests/test_constants_sync.py +++ b/tests/test_constants_sync.py @@ -33,21 +33,6 @@ def _as_str(value: object) -> str: return str(value) -# --- Numeric boundaries --------------------------------------------------- - -SHARED_INTS: dict[str, int] = { - "MIN_PASSWORD_CHARS": 8, - "MAX_PASSWORD_CHARS": 64, - "MIN_PASSPHRASE_WORDS": 3, - "MAX_PASSPHRASE_WORDS": 10, - "MIN_USERNAME_LENGTH": 3, - "MAX_USERNAME_LENGTH": 64, - "MIN_USERNAME_WORDS": 1, - "MAX_USERNAME_WORDS": 3, - "_MIN_PASSPHRASE_ENTROPY_BITS": 50, -} - - class TestConstantsSync: """Verify every shared constant matches between ``constants`` and ``generator``.""" @@ -110,4 +95,4 @@ def test_passphrase_specials(self) -> None: # --- Constant collections (lists / frozensets) ----------------------- def test_default_wordlist(self) -> None: - assert constants.DEFAULT_WORDLIST == generator.DEFAULT_WORDLIST \ No newline at end of file + assert constants.DEFAULT_WORDLIST == generator.DEFAULT_WORDLIST From 58b74e9ac410db3acffe6cce80f538a9c07f2c34 Mon Sep 17 00:00:00 2001 From: j-kemble Date: Mon, 17 Aug 2026 17:23:59 -0500 Subject: [PATCH 3/3] docs(test): document policy-tuple duplication + constants single-source-of-truth debt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documents the two known-debt items acknowledged in the PR #3 remediation round-2 review (review-APPROVED-pr3-remediation-round2-20260817-2217.md §6): CLIPBOARD_AUTO_CLEAR_OPTIONS / AUTO_LOCK_OPTIONS are defined independently in tui.py and constants.py (byte-identical, unguarded), and the intended fix is a single-source-of-truth migration making generator.py and tui.py import from constants.py. Cosmetic only; no behavior change. --- tests/test_constants_sync.py | 45 +++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/tests/test_constants_sync.py b/tests/test_constants_sync.py index 72ac4a1..553e814 100644 --- a/tests/test_constants_sync.py +++ b/tests/test_constants_sync.py @@ -1,16 +1,35 @@ -"""Test that every constant duplicated in ``constants.py`` matches its -counterpart in ``generator.py``. - -The duplication between ``generate_it/constants.py`` and -``generate_it/generator.py`` is a known architectural debt — the former was -extracted as a central registry after the latter already defined many module- -level constants. This test guards against silent drift (see -``review-REJECTED-consolidated-remediation-20260816-2305.md`` §3, which caught -``USERNAME_ALPHANUMERIC`` having diverged). - -If this test fails, the two modules are out of sync. Fix the source of truth -(first decide which one it is, then update the other) and add the correction -to this test so future drift is caught. +"""Test that every constant duplicated across modules matches its counterpart. + +This test currently guards a single duplication surface: + +**``constants.py`` ↔ ``generator.py``** — generator-level constants +(length bounds, character sets, wordlists, entropy minimums). This is the +original drift surface; ``USERNAME_ALPHANUMERIC`` was caught diverging here +(``review-REJECTED-consolidated-remediation-20260816-2305.md`` §3). + +If a test here fails, the two modules are out of sync. Fix the source of +truth (first decide which one it is, then update the other) and add the +correction to this test so future drift is caught. + +-------------------------------------------------------------------- +Known architectural debt — not yet guarded by this test suite +-------------------------------------------------------------------- + +**Policy-tuple duplication** — ``constants.py`` ↔ ``tui.py`` + ``CLIPBOARD_AUTO_CLEAR_OPTIONS`` and ``AUTO_LOCK_OPTIONS`` are defined + independently in both modules (``tui.py`` at lines 52, 62 is the runtime + authority; ``constants.py`` at lines 10, 20 is imported by + ``tui_render.py``). These are verified byte-identical at the time of + PR #3 remediation but are *not* guarded by a sync test — they could + silently diverge. + +**Constants single-source-of-truth migration** + Making ``generator.py`` and ``tui.py`` import from ``constants.py`` + (rather than defining independent copies) is the intended resolution for + *all* duplication surfaces above. The sync test is a pragmatic interim + guard; the full migration is deferred for scope-risk reasons. When the + migration lands, this file should be updated to verify the *remaining* + guards rather than the duplication surfaces. """ from __future__ import annotations