Skip to content

refactor(public-safety): own the lowercase public-safe slug shape - #5360

Open
karenchuu wants to merge 2 commits into
loopx-project:mainfrom
karenchuu:codex/public-safe-slug-owner
Open

karenchuu wants to merge 2 commits into
loopx-project:mainfrom
karenchuu:codex/public-safe-slug-owner

Conversation

@karenchuu

Copy link
Copy Markdown
Contributor

Goal And Delivered Outcome

  • Outcome basis / anchor: no pre-existing issue. The anchor is a measurement on the intended base a9ee074de: the shape ^[a-z][a-z0-9_.-]{0,127}$ -- "is this identifier a lowercase public-safe slug" -- was compiled by seven modules themselves: capabilities/periodic_report/core.py:17, capabilities/issue_fix/periodic_report.py:20, control_plane/handoff/review_batch.py:15, capabilities/periodic_report/adapters.py:28, capabilities/periodic_report/archive.py:27, capabilities/periodic_report/audience.py:17 and capabilities/periodic_report/bindings.py:27. Consequence: tightening or loosening the slug rule was a seven-file change, and four of the seven lived in one package where the others could not see them.

  • Observable before → after: loopx/public_safe_text.py states the shape once as PUBLIC_SAFE_SLUG_PATTERN, and the three modules no open branch is editing ask it for the answer -- the other four are declared by file and count in the guard, not silently skipped. Product side is 4 files, +14 / -8: three compiled copies and two now-unused import re lines leave, one owner line and four imports come in. Every accepted and rejected value stays the same, which the guard's accept/reject corpus and 48 pre-existing test files confirm.

  • Why this owner and not a new module: the shapes a public-safe field may hold already live in public_safe_text.py (the module docstring calls it the canonical private-text owner, and refactor(public-safety): centralize compact identifier shapes #5351's PUBLIC_SAFE_REFERENCE_PATTERN set the precedent of putting an allowlist shape there), it is reachable from control_plane/handoff and both capability packages without crossing test_control_plane_import_boundaries.py, and it holds zero census rows. A new top-level module was not available at all: tests/architecture/top_level_module_budget.json allows 147 and loopx/*.py is at exactly 147.

  • Intended base: a9ee074de.

Scope And Continuation

  • Done: the owner constant, three converted modules, one 41-case guard.
  • Declared, not hidden -- the four files whose import blocks an open branch of mine (refactor(digest): one owner builds the stored SHA-256 envelope #5337) is rewriting: adapters.py, archive.py, audience.py, bindings.py. Converting them in this slice would have created a conflict between two pending pull requests of the same author for no semantic gain. DECLARED_INDIVIDUAL_SITES pins one site per file, and the guard fails in both directions: a second copy appearing there, and a site converted without its entry retired. Successor: delete four entries when refactor(digest): one owner builds the stored SHA-256 envelope #5337 lands.
  • Not collapsed, asserted as probes that must not be reported:
  • Deliberately per-surface: each converted module keeps its own _token-style helper and its own error text, because those describe the owning surface, not the shared rule -- the same split public_safe_text.py documents for the four text validators.
  • Slice boundary / successor: complete within scope; reverting is three imports and one constant.

Validation

  • Tested revision: c804d5c9f (2 commits, 5 files, +484 -8).
  • Run state: finished.
  • Input classes: synthetic fixtures; the affected scope also runs the CLI-subprocess quota family.
Check kind Result Public-safe evidence / limitation
unit passed pytest tests/architecture/test_public_safe_slug_owner.py -> 41 passed in 6.4s: value scan over loopx/ with anchor normalization and same-file constant folding, declared-site counts in both directions, per-consumer identity plus a real reference, 7 accept / 14 reject cases including the 128/129 boundary, 9 spelling probes (6 that must be reported, 3 that must not) and the unfoldable-declaration probe.
integration passed pytest tests/architecture tests/canary in full at this head -> 1112 passed, 0 failed in 2m04s. This branch adds a module-level constant to a file that several architecture guards read by name, so the whole pair was run rather than a slice.
regression_parity passed pytest over the 48 test files that mention periodic_report, review_batch or public_safe_text -> 1204 passed, 1 failed in 2m51s. The one failure is tests/control_plane/test_quota_settlement_cli.py::test_read_only_settlement_omits_non_causal_delivery_workspace. Attribution, run under identical conditions on an unmodified a9ee074de worktree and on this head, alternating: the unmodified base failed that same file in round 1 (1 failed, 78 passed) while head passed both rounds (79 passed, 79 passed), and the node id alone passed 3/3 on each tree. It is a quota/heartbeat subprocess case that reads shared local state, so it flips with order and load; this branch adds nothing it depends on.
static passed python -m ruff check on all five changed paths: clean (it is also what caught the two import re lines left unused after the migration). python -m mypy (no arguments, as CI runs it): Success: no issues found in 19 source files. git diff --check: clean.
static passed loopx check --scan-path for each of the five changed paths through this tree's own entrypoint: ok: true, "public boundary scan clean: 5 files"; both warnings concern the absent local .loopx/registry.json. One non-literal credential reference was downgraded, as it is on the unmodified base.
semantics budget passed examples/semantic-vocabulary-drift-smoke.py on the unmodified base worktree and on this head, same venv, same Node 22.23.2, same node_modules: output byte-identical, conflicting_definitions=55/55, conflicting_values=16/16. As in the benchmark slice, a re.compile value is not one of the inventory's counted kinds, so the new constant name enters no budget and no anchor needed editing; the name itself is unique in the tree (0 prior hits).
canary passed loopx canary premerge with the five changed files passed explicitly: selected 18 / executed 18 / failures 0 / warnings 0, status: passed, no manual holds.
mutation passed 11 mutations, one at a time in a separate worktree at this head, all files restored before every round, control round green (41 passed) before and after: 10 caught / 1 survived. Caught: a converted consumer restating the regex and calling it (M1, 7 cases incl. the periodic-report suite); a half-anchored restatement ^[a-z][a-z0-9_.-]{0,127} (M2, 3 cases -- the scan normalizes anchors because they are redundant under fullmatch); a shape assembled from two same-file constants (M3); an inline re.fullmatch inside a function (M4); the owner's bound tightened by one character (M5); the owner's class allowing one more character (M6); a consumer keeping the import while deciding with a weaker local check (M7); a declared site gaining a second copy without retiring its count (M8, 3 cases); an unfoldable construction in a module that mentions the class (M9); a validator helper restating the shape inside a converted module (M10, 3 cases). Disclosure: M9 and M10 first reported survived, and the cause was my driver, not the guard -- two edit() calls on one file each rewrote from the pristine copy, so the second silently discarded the first. Fixed by making each mutation one edit, then both were caught. Survived, and why it is equivalent: M11 adds token.islower() alongside the owner check; for every value the pattern accepts that predicate already holds, and the periodic-report suite stayed green, so it is not a second owner.
frontend none No UI, projection or rendered surface changes.

Type Of Change

  • Internal refactor / single-owner convergence with an anti-regression guard. No behaviour change.

LoopX Area

  • loopx/public_safe_text.py, loopx/capabilities/periodic_report/core.py, loopx/capabilities/issue_fix/periodic_report.py, loopx/control_plane/handoff/review_batch.py, tests/architecture/.

Technical Direction

  • Seven copies of one shape is the pattern this repository already rewards with a guard: the decision, not the spelling, is what gets pinned. Anchors are normalized because fullmatch makes them redundant, which is exactly the case a text-diff review would wave through.
  • Staged boundary: four sites are declared because a sibling branch owns their import blocks. The declaration is machine-checked, so the boundary cannot quietly become permanent.

Boundary Checklist

  • Neither the diff nor this PR body/comments/attachments disclose private state, credentials, raw traces or verifier output, internal links, or local machine paths.
  • I did not duplicate maintainer-owned benchmark work unless a maintainer split out a public issue for it.
  • I kept the change scoped to the linked issue/task.
  • I completed the visual evidence section for UI changes, or marked UI impact none.

Seven modules compiled the same slug shape themselves. This converts the three
that no other open branch is editing: the periodic-report request core, the
issue-fix report source and the hand-off review batch. Two of them also dropped
a now-unused import re.

The canonical private-text module is the owner because the shapes a public-safe
field may hold already live there, and control_plane plus two capability
packages can reach it without crossing the import-boundary guard.

Signed-off-by: karenchuu <25980598+karenchuu@users.noreply.github.com>
The scan folds a pattern through same-file string constants and normalizes
anchors, so a half-anchored or assembled restatement is reported the same as a
literal copy. A construction whose value cannot be folded must be declared with
its file and count.

Four periodic_report files keep their own copy, declared by file and count:
their import blocks are being rewritten by an open branch, so converting them
here would only create a conflict between two pending pull requests. Both
directions of each count are checked, so converting one without retiring its
entry fails.

Signed-off-by: karenchuu <25980598+karenchuu@users.noreply.github.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant