fix: a docstring above a mark hid its coverage, and attribute failed inside somebody else's package - #33
Merged
Conversation
…inside somebody else's package
Eleven usage patterns, tried against the package rather than read out of it — the record file
round-trip, counting across threads, a run that threw, the mark's exit, `reach_script` on a real
script, a method on a foreign generic, the verify layer, the timing half, the documentation
helpers, records merged across processes, and a consumer of a marked package.
Nine came back correct. Two did not.
## `verification` reported `missing` for the shape this package recommends
bare_mark covered=1 total=1 fraction=1.0
documented_mark covered=0 total=0 fraction=missing
Same file, same process, each entered once. The only difference is a docstring above the mark.
`"docstring"` above a definition parses into ONE statement — `Core.@doc "…" <definition>` — whose
start line is the DOCSTRING's. `_find_span` matched a sibling starting exactly on `mk.line`,
nothing started there, and the fraction came back `missing`: "no information", for a definition
that had just run forty times with a `41` sitting next to it in the `.cov` file.
It matters because it is the *recommended* shape. `audit` asks every public name for a docstring
and every unfinished one for a mark — "they are not alternatives; the docstring is owed either
way" — so a package that follows the advice got the broken half. `unverified` was unaffected: it
reads the probe first, which is exact.
The spec now pins it with the control in the same run — bare and documented, both entered, same
fraction — and was checked against the reverted fix: 2 failures with it out.
## `attribute(record)` failed inside `Profile`
MethodError: no method matching getdict(::ExperimentalAPI.Record)
`record` hands back a `Record`, `attribute` is the neighbouring verb, and `attribute` takes a
profile *buffer*. The mistake is the one this API sets its user up for, and the message named
neither the function they wrote nor the argument they passed. It now says what it got, what a
`Record` already carries, and what to pass instead.
## And one thing stated rather than fixed
`read_record` resolves `Hit.mod` by name: present in this process, it comes back pointing at the
module; absent — the ordinary case when shards are merged on a machine that never loaded the
package — it falls back to `Main`, and the report reads `Main.energy` for a name that does not
exist in `Main`. `Hit.mod` is a `Module` and a file cannot carry one. Documented next to the
`method`/`value` note that was already there, and pinned in both directions.
193 behaviours, 1253 assertions, green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
📚 Docs preview: https://codes.sota-shimozono.com/ExperimentalAPI.jl/previews/PR33/ (updates on each push to this PR) |
34 comment lines added by the previous commit, 13 now. What is left is the measurement and the parse fact that is not visible from the code; what went is the narration around them.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
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.
Eleven usage patterns, tried against the package rather than read out of it: the record file
round-trip, counting across threads, a run that threw, the mark's exit,
reach_scripton a realscript, a method on a foreign generic, the verify layer, the timing half, the documentation
helpers, records merged across processes, and a consumer of a marked package.
Nine came back correct. Two did not.
1.
verificationreportedmissingfor the shape this package recommendsSame file, same process, each entered once. The only difference is a docstring:
bare_markdocumented_mark"docstring"above a definition parses into one statement —Core.@doc "…" <definition>—whose start line is the docstring's.
_find_spanmatched a sibling starting exactly onmk.line, nothing started there, and the fraction came backmissing— "no information" — for adefinition that had just run forty times, with a
41sitting next to it in the.covfile:It matters because it is the recommended shape.
auditasks every public name for a docstringand every unfinished one for a mark — "they are not alternatives; the docstring is owed either
way" — so a package following the advice got the broken half.
unverifiedwas unaffected: itreads the probe first, which is exact.
Pinned with the control in the same run (bare and documented, both entered, same fraction), and
checked against the reverted fix: 2 failures with it out.
2.
attribute(record)failed insideProfilerecordhands back aRecord,attributeis the neighbouring verb, andattributetakes aprofile buffer. This is the mistake the API sets its user up for, and the message named neither
the function they wrote nor the argument they passed. It now says what it got, what a
Recordalready carries, and what to pass instead.
3. One thing stated rather than fixed
read_recordresolvesHit.modby name. Present in this process → it points at the module; absent— the ordinary case when shards are merged on a machine that never loaded the package — it falls
back to
Main, and the report readsMain.energyfor a name that does not exist inMain.Hit.modis aModuleand a file cannot carry one. Documented next to themethod/valuenotealready there, and pinned in both directions with a real subprocess.
The nine that were correct
record file round-trip (hits, counts, reasons, versions preserved;
valuenothing) · threads(4000 counts across 4 threads, exact) · a failed run (
rethrow = falsereports what it entered) ·until=/promotable/marks_without_exit/stale_since/exceeds_mark_cap·reach_scripton a real script (:depends, names the mark) ·Base.showon a foreign type(
contributed_methods,compare_methods, rendering unaffected) ·assert_cleanboth ways ·snapshot file round-trip and
isbreakingboth ways · cross-process merge (10 + 25 = 35).193 behaviours, 1253 assertions, green.
🤖 Generated with Claude Code