docs(http): fix three stale measurements in the guess_version fallback comment

[JarryShaw]

Please follow the guide below

• Searched for similar pull requests
• Followed the coding style (make pylint, make mypy, make isort)
• make test passes, and a test case covers the change
• Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible

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

Closes #824

The _guess_version comments (roughly :310-405) cited measurements from
before #799 and #811 landed. Rewritten past tense with the re-measured,
current outcome, keeping each suppression/conversion's causal explanation
intact:

• HTTP(io.BytesIO(b'\x00' * 8), 8) used to raise a bare struct.error; it
  now raises ProtocolError: unknown HTTP version via httpv2: the frame guard tests the declared length, not the buffer, so a 4-octet frame can report length=16777215 #799's guard.
• A 16-octet GOAWAY was cited (in two places — the fallback arm and the
  preface arm's "corekit: a negative field length only warns in Schema.unpack, instead of raising ProtocolError #805 residual" note) as still raising a bare
  struct.error through httpv2.HTTP directly. fix(fields): raise ProtocolError, not struct.error, on a negative resolved field length #811 closed that at
  FieldBase.length; the same input now raises ProtocolError: Field debug resolved to a negative length; template='-1s'.
• Restored the dropped note that fix(fields): raise ProtocolError, not struct.error, on a negative resolved field length #811 deliberately left Schema.unpack's
  own running-counter warning alone (converting it would reject a
  SETTINGS frame with a short trailing entry that parses successfully
  today while only warning) — that SchemaWarning still fires.
• Reworded the last arm's "stays regardless" claim to attribute the
  decision to fix(fields): raise ProtocolError, not struct.error, on a negative resolved field length #811 and point at fix(fields): #811 misreports a malformed template as a negative length, and never prevents the negative length #825 as the open question, rather than
  asserting permanence.

Comment-only, no behaviour changed — confined to
pcapkit/protocols/application/http.py. Confirmed 21 code objects, 0
structurally differing (bytecode, non-code constants, names, def line
numbers). N/A — changelog centralised in #657.

Ran pylint/mypy/isort scoped to this file (clean; pylint's 5
pre-existing findings are all outside the changed lines). Did not run
make test (full suite OOMs); ran pytest tests/protocols/application/:
123 passed, 0 failed, 432 subtests. No test covers this change since it is
comment-only.

[JarryShaw]

Rebased onto main (21e9588af) and force-pushed: 2448c962a → 8e11568b2.

It was BEHIND after two merges to main; both were test-only and touch nothing this PR does. Content-identical rebase — diff of the pre- and post-rebase diffs is empty, still one commit and only pcapkit/protocols/application/http.py.

Still review: pending — this head has no verdict yet, and a cross-review is what it needs next. Note #821 also touches this file (one comment line), so one of the two will need a rebase after the other merges.

[JarryShaw]

Cross-review verdict: GOOD TO GO on what it changed (haiku; author was sonnet) — but I am holding it at review: needs-changes for two prose items in the same function, both of which I verified. This PR exists to stop stale comments; shipping a new one would be perverse.

1. The function now contradicts itself, and #826 is the right place to fix it. :313-314 still reads "a preface followed by a frame that trips the #805 residual (an inner field shortfall, e.g. a 16-octet GOAWAY)" — present tense, while #805 is CLOSED/COMPLETED and this PR's own new text 60 lines below says #811 closed that class, citing the same 16-octet GOAWAY. It sits outside both hunks (@@ 359,16 @@ and @@ 383,15 @@), so it was simply out of range. No collision risk: http.py is off-limits to #825's worker by its brief.

2. The PR's own new prose asserts permanence about the line #825 may remove. :372-375: "Suppressing struct.error on the last arm stays regardless, as defence in depth." True today, stale the moment #825 narrows or deletes that suppression. Attribute the decision and point at the open question instead.

The review's own verification was strong and I am not re-litigating it: all three target measurements reproduced exactly, including the GOAWAY's string-identical message with __cause__ a struct.error; the unflagged sentences checked too (#799's httpv2.py:251/:254/:311 guards, the last-arm-only suppression at :416/:420/:423, and the unedited "450 times out of 450" re-derived as 225+225); comment-only proven by compiling both sources — 12 code objects, 0 of 12 differing in co_code, co_consts, co_names or co_firstlineno, with two injected mutations confirming the comparison can fail; tests/protocols/application/ now 123 passed, 0 failed, 432 subtests (better than the author reported, because the rebase picked up #822's fix); pylint 5 findings at 9.41/10, identical on the origin/main control, all outside the edit range. "As defence in depth" really is #811's own wording.

One correction to its method, which matters because it would have discredited other measurements: it reported that sys.path.insert alone was insufficient and that the editable finder had to be evicted from sys.meta_path. Measured — _EditableFinder is appended after PathFinder, so sys.path.insert(0, ROOT) alone resolves correctly. The eviction was harmless, not necessary.

One loss worth a clause while you are in there: the old text named Schema.unpack alongside FieldBase.length, and #811 deliberately fixed only the latter — its commit says converting the other "would reject a SETTINGS frame with a short trailing entry, which parses successfully today while only warning." That recorded "we chose not to" is gone, and the SchemaWarning still fires.

[JarryShaw]

All three items are in at 198d0b3a8, verified by me, and the diff is still comment-only.

git diff --stat origin/main..198d0b3a8      -> 1 file, +40/-23
non-comment changed lines                    -> 0
parent                                       -> 21e9588af (current main)

1. :310-317 no longer calls the corekit: a negative field length only warns in Schema.unpack, instead of raising ProtocolError #805 residual live — it now reads "fix(fields): raise ProtocolError, not struct.error, on a negative resolved field length #811 has since closed that residual at …", past tense, while keeping why the except (ValueError, struct.error) clause still exists.
2. The permanence commitment is gone. grep still finds the words "stays regardless", which would have flagged wrongly — read in context it now means stays regardless of that route being closed, attributes the decision to fix(fields): raise ProtocolError, not struct.error, on a negative resolved field length #811, and names fix(fields): #811 misreports a malformed template as a negative length, and never prevents the negative length #825 at :381 as the open question, "not settled here."
3. The Schema.unpack clause is restored at :404-405, with fix(fields): raise ProtocolError, not struct.error, on a negative resolved field length #811's own reason for leaving that warning alone.

So the whole function is now internally consistent about #805, which is what #824 was filed for. tests/protocols/application/ 123 passed / 0 failed / 432 subtests; isort, mypy and pylint clean with the same 5 pre-existing findings outside the edit range.

CI has just restarted on the new head. Verdict basis, stated plainly: the cross-review said GOOD TO GO on the original three measurements, and these three additional items were specified by me and verified by me — they have not been through an independent reviewer. For comment-only prose with a proven-identical compiled module that is proportionate; I would not do the same for a code change. review: good-to-go goes on once CI reads inc=0.

[JarryShaw]

Rebased onto main (c287152d7) and force-pushed: 198d0b3a8 → dd16b8f85. This was the collision flagged earlier — #821 merged and touches this same file, so the previous green CI described a tree that no longer existed.

The rebase is clean and the change is content-identical, but the raw diff is not, which is worth spelling out: #821's hunk at @@ -337,8 +337,9 @@ sits between this PR's hunks at @@ -310 @@ and @@ -359 @@, so every @@ offset moved. Comparing only the added and removed content lines:

diff <(git diff 21e9588af...198d0b3a8 | grep -E '^[+-][^+-]') \
     <(git diff c287152d7...dd16b8f85 | grep -E '^[+-][^+-]')
  -> CONTENT IDENTICAL (only @@ offsets moved)

So the GOOD TO GO verdict carries. Spot-checked the merged state: #821's line survives at :346 (#682 now routes 231 real HTTP/1 frames), this PR's #811 has since closed that residual at :316, its #825's reference at :382, and both Schema.unpack mentions intact.

One process note against myself: my first check of that #811 has since closed… string returned 0 and I nearly reported it missing — I had omitted the filename, so grep -c read empty stdin. Re-run with the file and a known-absent control string, it is 1 and the control is 0. Exactly the unvalidated-probe failure I brief reviewers about.

CI restarts on dd16b8f85; review: pending until it reads inc=0.

[JarryShaw]

review: good-to-go on dd16b8f85 — CI is complete and clean, ok=27 fail=0 inc=0. That is the last gate; this is ready to merge.

The verdict basis, restated plainly so it is not overstated: the cross-review said GOOD TO GO on the three original stale measurements, and the three further prose items were specified by me and verified by me — not by an independent reviewer. Proportionate for a comment-only change whose compiled module was proven identical (12 code objects, 0 differing in co_code/co_consts/co_names/co_firstlineno, with two injected mutations confirming the comparison can fail). I would not apply that standard to a code change.

The rebase onto c287152d7 was content-identical — only @@ offsets moved, because #821's hunk landed between this PR's two — so the verdict carries across it.

One ordering note now that #821 has merged: the collision is resolved in this PR's favour. It was the second of the pair to rebase, so it already contains #821's line at :346 alongside its own at :316 and :382. Nothing further is needed before merging.