Skip to content

docs(tests): stop crediting the owner with wording he never wrote - #994

Merged
JarryShaw merged 3 commits into
mainfrom
docs/987-false-attributions
Oct 2, 2026
Merged

JarryShaw merged 3 commits into
mainfrom
docs/987-false-attributions

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • chore — anything else

Description of your pull request and other information

Part of #987, and a worse defect than the quotations that issue usually removes: five sites presented an agent's own phrasing as your ruling. A quotation you did write is merely against convention; a quotation you did not write is a fabricated citation.

The evidence. A fully paginated search of all ~2117 issue and pull-request comments, plus every inline review comment, finds renaming anything exactly once — in the #982 review that first reported this very invention — and finds who may claim this pool nowhere. a real ownership fact occurs only in an agent's own analysis on #775 (comment 5859210283, 3471 characters), and there it describes the Xerox row in the IPX socket registry, not Xyplex.

The substantive correction. test_const_enum_no_mint.py gave the wrong reason for keeping Xyplex minted. Your actual reason, on #775 at 19:53:45Z, is that a proprietary protocol has no public name of its own so the company name serves as one. Four sites carried the agent's gloss instead; all four now carry either that reason or your mint-versus-notation criterion from #847. test_const_ethertype_862_unit.py no longer credits a ruling with the scoping — PR #878's body is where that came from, so the prose says so.

#982 found this and named four lines; it was never fixed, and the sites had since drifted by nine. Where the phrase legitimately survives it is now unquoted and credited to #878.

A correction to my own method, since I cited the search as evidence on #993. My first sweep used ?per_page=100&page=N over six pages — 600 of ~2117 comments, under a third — and missed the agent comment that actually contains ownership fact. gh api --paginate is the only exhaustive form. The conclusion is unchanged and in fact better supported, but the search I described was not the search I had run.

Prose only. With comments and NL dropped the token sequences are identical at 10751 each; the AST with docstrings blanked compares equal in both files; test_const_ethertype_862_unit.py is identical once comments are masked; no assertion depends on any changed text; and neither file gains a line over 95 characters.

@JarryShaw JarryShaw added docs Pull requests that change documentation only (docs: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one test Pull requests that add or correct tests (test: subject prefix) labels Oct 2, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 0e03a593e — opus cross-review. No blocking finding. Four corrections to my own account first, two of them to things I published.

My cited source was wrong, in the same class of way as the error I corrected on #993. I said the phrase occurs in comment 5859210283. It does not — that comment says "(an ownership fact)", a variant. The exact phrase originates in PR #861's body, line 19. My search covered comments but not bodies, and the ~989 issue and pull-request bodies are part of the corpus — #987's own body says so. Having just corrected an incomplete pagination, I then ran an incomplete corpus. And when I went to check the reviewer's claim, my own grep missed the phrase in #861 because I required 30 trailing characters where only 26 follow. Three mechanical misses in one verification chain; the conclusion survived each time, which is exactly what makes them worth stating.

#847 is the primary source and I cited only #775. The reason for keeping the company name was given on #847 at 13:15:36Z, 190 characters, six and a half hours before #775's 19:53:45Z confirmation. Citing both is right; citing only the later one credits the wrong thread.

"All five sites are fixed" overstates — a sixth survives in production source. pcapkit/vendor/ipx/socket.py:256 carries the identical defect with an explicit attribution, crediting a ruling on #775/#841 with holding a row out as "a real ownership fact". That is a vendor template, so it needs the paired-edit treatment rather than a hand fix, and it is outside a docs(tests): tranche. Filing it rather than widening this one.

One objection I am recording rather than acting on. The replacement prose lifts genuine maintainer wording verbatim but unquoted — "the company one", "dynamically/statically assigned", "a final concrete assigned name", "notation for the reader". A change that removes a fabricated quotation substitutes real unmarked fragments, and documentation.rst:184 has no tests/ carve-out. I judge it non-blocking at three to five words with nothing asserting on them, but a stricter reading could object.

One thing I am fixing, because it is an inconsistency inside this commit: :600 drops "anything" while :1299 and :1549 keep it, and fidelity to #878 — which says "rather than renaming" — motivates all three equally. #878 is also credited with a sentence it did not write, since its body says "existing hex-suffixed name" where the survivors say "existing name argument exactly as the current code produces it".

Verified independently: a fully paginated corpus of 2118 comments, 334 inline comments and 989 bodies, plus review summary bodies on 14 pull requests — a category neither of my endpoints covers. No maintainer hit for any of the three phrases. Prose-only reproduced exactly at 10751 tokens, both differing strings docstrings on both sides, zero non-docstring differences, the over-95 line set identical rather than merely not grown, and reflow clean at each block's own wrap width.

Label goes to review: pending while the one-word fix lands.

@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 2, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Fixed and pushed as c4a26a94c. All four #878-crediting sentences now use #878's own wording.

There were four, not the three I listed — the module docstring carried one too, and the worker found it. That is the third time on this issue a count I supplied was short: 22 spans against 29 actual, five sites against six, three sentences against four. Each time the worker measured and I had not, which is the pattern rather than the exception by now.

On the substance it made the call I asked it to weigh rather than take from me: the longer phrasing carried nothing #878 lacked, and the prose at the fourth site already states the crawler-rendering point in the file's own voice, so dropping it loses no checked fact. The stray "about" and the trailing "anything" are gone, so all four agree.

Verified by me, and by word-level diff rather than token counts alone, which is the check that actually settles a re-flow: against the previous head the 68 docstrings show exactly the intended operations and nothing else, and test_const_ethertype_862_unit.py is untouched this round. With comments and NL dropped the file is token-identical to main at 10751, the AST with docstrings blanked compares equal, and the over-95 line set pairs up by length with no entry added.

One join reads like a wording change and is not, which is worth stating because it would be easy to mistake: still- ending a line and minting starting the next became still-minting. I checked main — the hyphen was already there, so this is a line join of an already-hyphenated word, not an added hyphen.

Label stays review: pending until the new head has its own verdict.

Part of #987, and a worse defect than the quotations that issue usually
removes: five sites presented an agent's own phrasing as the maintainer's
ruling.

Two phrases were attributed to him and appear in no comment anywhere. A fully
paginated search of all ~2117 issue and pull-request comments plus every inline
review comment finds "renaming anything" exactly once -- in the #982 review
that first reported this very invention -- and finds "who may claim this pool"
nowhere at all. "a real ownership fact" occurs only in an agent's own analysis
on #775 (5859210283, 3471 characters), and there it describes the Xerox row in
the IPX socket registry, not Xyplex.

- test_const_ethertype_862_unit.py no longer attributes the scoping to a
  ruling. PR #878's body is where it comes from, so the prose says so.
- test_const_enum_no_mint.py's Xyplex comment gave the wrong reason. The
  ruling's own reason, on #775 at 19:53:45Z, is that a proprietary protocol has
  no public name so the company name serves as one. Four sites carried the
  agent's gloss instead; all four now carry the real reason or the maintainer's
  mint-versus-notation criterion from #847.

#982 found this and named four lines; it was never fixed, and the sites had
since drifted. Where the phrase survives it is now unquoted and credited to PR
#878, which is what wrote it.

Prose only: with comments and NL dropped the token sequences are identical at
10751 each, the AST with docstrings blanked compares equal in both files,
test_const_ethertype_862_unit.py is identical once comments are masked, and no
assertion depends on any changed text. No file gains a line over 95 characters.
The cross-review found the surviving sentences crediting PR #878 with wording
#878 never used, and this branch doing it two ways. #878's body says the change
is about not registering rather than renaming, and that the existing
hex-suffixed name is preserved.

There were four such sentences, not the three I had listed -- the module
docstring carried one too. That is the third time in this issue a count I
supplied was short: 22 spans against 29, five sites against six, three
sentences against four. Each time the worker measured and I had not.

- All four now say what #878 says: keep the existing hex-suffixed name, since
  the change is about not registering rather than renaming. The longer form
  carried nothing #878 lacked, and the prose at the fourth site already states
  the crawler-rendering point in the file's own voice.
- The stray "about" and the trailing "anything" are gone, so the four agree.

Prose only, proven by word-level diff rather than token counts alone: against
this branch's previous head the 68 docstrings show exactly the intended
operations and nothing else, and test_const_ethertype_862_unit.py is untouched
this round. With comments and NL dropped the file is token-identical to main at
10751, the AST with docstrings blanked compares equal, and the over-95 line set
pairs up by length with no entry added.

One join reads like a wording change and is not: "still-" ending a line and
"minting" starting the next became "still-minting". The hyphen was already
there on main.
@JarryShaw
JarryShaw force-pushed the docs/987-false-attributions branch from c4a26a9 to 2bfd3e0 Compare October 2, 2026 19:46
The round-2 review found the reflow had joined a split cross-reference without
repairing it: main wrapped :func:`_is_hex_suffixed_ / unregistered_name` across
two lines, which renders as one space, and the reflow pulled it onto one line
keeping the space. That is worse than the artefact it replaced -- a wrapped role
reads as a wrapping accident, a one-line one reads as the name.

- The role is one unbroken name again. The function is at :1175 and was already
  cited correctly at :162.
- Two further roles the earlier reflows had split the same way are repaired:
  EnumRegistry._unregistered_member, which read "enum. EnumRegistry", and the
  pair separated by an oblique, which carried a space before it.
- The four #878 clauses now use one wording. They differed as keep against
  keeping and this way against that way; #878's body uses the gerund, so
  keeping is the closer form. The commit that claimed the four agreed is now
  telling the truth.
- The 80-column line in the Xyplex comment block is rewrapped. That block's
  width is 78.

Measured rather than asserted: zero single-line role targets contain a literal
space in either file, against zero on main, so the defect this round introduced
is gone and none was left behind. The normalised clause occurs four times and
none of the three former variants occurs at all.

Prose only: with comments and NL dropped both files are token-identical to main
at 10751 and 753, the AST with docstrings blanked compares equal, and the 24
over-95 lines pair up by length with nothing added.
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 3b3578f67 — round 2's opus review returned GOOD TO GO at the previous head, and the only delta since is the three non-blocking items that review itself named, each verified by me. I am granting on that basis rather than spending a fourth round on "were three spaces deleted correctly".

The review caught a defect this pull request introduced, which is the finding worth keeping. main wrapped :func:`_is_hex_suffixed_ / unregistered_name` across two lines; the reflow pulled it onto one line keeping the space. That is worse than the artefact it replaced — a wrapped role reads as a wrapping accident, a one-line one reads as the name. Two further roles the earlier reflows had split the same way are repaired alongside it.

Measured rather than asserted, and the measurement needed fixing first: my own sweep flattened newlines, so it counted every role spanning a line break as broken and reported 42 on main. The real test is a space inside a role target on a single line — zero in either file at this head, against zero on main. So the defect this round introduced is gone and none was left behind.

The four #878 clauses now genuinely agree: four occurrences of one wording and zero of the three former variants, measured on flattened text. My earlier commit claimed they agreed when they differed as keep against keeping and this way against that way. #878's body uses the gerund, so the normalised form is the closer one.

Two corrections to my counts from the review, the fourth and fifth on this issue. There are five #878-crediting sentences across the two files, not four — the fifth is in test_const_ethertype_862_unit.py from round 1 and already uses #878's own wording, so nothing survived unaligned. And the four are a module docstring, a #: comment, a class docstring and a method docstring, not two method docstrings. My token figures are also one low per file, 10752 and 754, because my drop set discards ENCODING as well.

One finding in the work's favour that I had not established: the phrase round 2 dropped appears nowhere in #878's body, so it was invented and over-claiming — #878 states explicitly that the vendor output was not byte-regenerated and that this is weaker than byte-identity. Removing it deleted a false claim rather than a checked fact.

Verified at this head: token-identical to main at 10751 and 753 with comments and NL dropped, the AST with docstrings blanked equal, every differing string token a docstring on both sides, no assertion depending on changed text, and the 24 over-95 lines pairing by length with nothing added.

Labelling review: good-to-go. Unpublished and unmerged — yours to merge.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 2, 2026
@JarryShaw
JarryShaw merged commit 0ac0c89 into main Oct 2, 2026
72 checks passed
@JarryShaw
JarryShaw deleted the docs/987-false-attributions branch October 2, 2026 20:01
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Pull requests that change documentation only (docs: subject prefix) test Pull requests that add or correct tests (test: subject prefix)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant