fix(masters): guard against an empty Metrika counter identity - #808
Merged
axisrow merged 2 commits intoAug 7, 2026
Conversation
_metrika_counter_identity returned an empty string when a counter's first line ended with a trailing " • " and nothing after it (e.g. a malformed or unexpected markup shape). Two different malformed inputs would both collapse to the same empty identity and falsely match each other in _verify_saved's Counter comparison, masking a real mismatch instead of failing to match anything -- the intended, safe failure mode this function already documented for the no-separator case. Found via cycle-review (/review, delayed report).
… trailing tokens cycle-review round 2 finding (Codex + /review, both independently): the exactly-empty guard added in this PR's first commit used bare truthiness, so a trailing " • " followed only by whitespace (e.g. "Label • ") still returned as a non-empty identity -- two different malformed inputs could again collapse to the same identity and falsely match in _verify_saved's Counter comparison, the exact failure mode this PR set out to close. Switch the guard to identity.strip() and add the whitespace-only mirrors of the existing empty-token tests.
Owner
Author
🔍 Local review (cycle 1) — round 88b93294-d40c-4fcd-95c1-126123914c34Reviewed locally (
Fixed in 1f4dd7a: guard now checks |
Owner
Author
📋 Review summary — all cycles
Totals: 1 FIX (resolved), 1 deferred to follow-up (#809). Codex round 2 re-review: approve, no material findings against the fixed diff. |
This was referenced Aug 7, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to PR #801 (Metrika counters in
masters update) — a fix that was pushed to that PR's branch after it had already been merged, so it never landed inmain. Cherry-picked here as its own PR against currentmain._metrika_counter_identity(added in #801's cycle-review to compare a counter's autocomplete-suggestion text against its read-back tag-display text by numeric id) returned an empty string when a counter's first line ended with a trailing" • "and nothing after it — a malformed/unexpected markup shape. Two different malformed inputs would both collapse to the same empty identity and falsely match each other in_verify_saved'sCountercomparison, masking a real save mismatch instead of failing to match anything (the safe fallback this function already documented for the no-separator case).Now returns the input text unchanged (instead of
"") when the last" • "-delimited token is empty.Test plan
pytest tests/test_masters.py -k "TestMetrikaCounterIdentity"— 5 passedpytest -n auto— 3416 passed, 23 skipped, no regressionsblack/flake8clean on all changed filesFound via cycle-review (local
/review, delayed report — round completed after PR #801 had already merged).Relates to #648.
🤖 Generated with Claude Code