Python citation block parser + shared citations.json fixture - #230
Open
jat255 wants to merge 7 commits into
Open
Python citation block parser + shared citations.json fixture#230jat255 wants to merge 7 commits into
citations.json fixture#230jat255 wants to merge 7 commits into
Conversation
Adds parse_commons_citation() and the CitationDecision record to the Python package, with the grammar pinned as a cross-language fixture that both suites run. The plan's Task 4 sketch disagreed with the R implementation on two points, and the R source wins because this is a contract between the two packages: - A wrapped quote (a blockquote run followed by an un-prefixed line) parses rather than failing as malformed. The continuation becomes explanation, so it is never verified. Nothing unverified is presented as verified. - The marker is `^> ?` with no indent allowance, so an indented `>` line is not evidence and its body is malformed. CitationDecision carries only the statuses R records (accepted, rejected, malformed); the plan also listed "missing", which nothing produces. On the R side this is tests only. The three hand-written parser tests are replaced by a runner over the shared fixture, which covers those cases and seven more, and a new test pins the commons.citation.candidates record shape that the trajectory reviewer reads.
roborev job 104 on 2ac4c9b, three low findings, all applied. CitationDecision is frozen like its siblings and now rejects a record that misreports its verdict: label and kind belong to an accepted decision only, and only a malformed one has no quote. The record is what the R trajectory reviewer reads back out of the span, so a wrong shape fails silently there. The fixture guard pins the set of statuses rather than a case count, so three accepted cases can no longer leave the omission logic uncovered. Both suites assert it. The explanation is trimmed with each language's own whitespace class, so a trailing U+00A0 survives in R and not in Python. Recorded in the fixture as an accepted divergence rather than engineered away: the explanation is never verified, so the difference is cosmetic.
roborev job 106 on e78039f, two low findings, both applied. Literal is not enforced at runtime, so a typo in a status reached as_record() and put a value on the span that the R reviewer cannot interpret. The class is the only place that can be caught, so it checks the vocabulary before checking the field combination. The half-populated accepted shape (a label with no kind) already raised; it is now pinned, because that is the shape a producer would build by accident when copying fields off a corpus entry.
roborev job 108 on 9730e3e. The vocabulary was written twice, so the annotation and the runtime guard could drift in either direction. CitationStatus now sits next to CitationKind and the guard reads get_args() off it.
roborev job 110 on c326257. kind was the last field of the record left unconstrained while its sibling CorpusEntry.kind already used CitationKind, so a kind the R reviewer cannot interpret could be serialized onto the span. Every field that reaches as_record() is now checked at construction. The guard belongs on the record rather than further upstream, because the record is what crosses into the trace.
|
Preview deployed to Connect ( Deployed from commit cc6a499. |
|
Preview deployed to Connect ( Deployed from commit cc6a499. |
Branching on the corpus rather than switching over the whole producer call expresses the same three assertions in a third less code. Coverage is unchanged: 217 expectations in the file before and after, four of them from this test.
The file repeated two different sample passages across ten literals. The tests that pin shared behavior read their inputs from tests/shared/citations.json; the rest only need some quote, so they now share one named sample, as the R suite already does with scanner_test_quote. test_matching_returns_the_entry_that_verified no longer builds a two-entry corpus. Which entry wins is pinned by the fixture's first-match-wins case, so restating it here was a per-language copy of shared behavior. The test keeps only its own claim, that the matched entry itself comes back.
jat255
added
r
jat255
marked this pull request as ready for review
September 1, 2026 00:46
Collaborator
Author
|
@simonpcouch this includes a minor change to R citation tests since they are using a the shared fixture with the Python side. It also added a few new tests to ensure citation formats match in both implementations. There's no functional changes in runtime R code, so it's all additive, but just an FYI. |
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
Adds
parse_commons_citation()to the Python package. The citation grammar and thecommons.citation.candidatesrecord move into a shared fixture (tests/shared/citations.json) that both suites read.R changes are tests only — no functional R code changed.
What changed
parse_commons_citation()and a frozen, self-validatingCitationDecisiondataclass. Status/kind vocabularies come fromLiteralaliases viaget_args();label/kindexist only for accepted decisions, and only malformed decisions lack a quote.tests/shared/citations.json.test-citation-scan.Rnow reads parser cases from the fixture viashared_fixture().trajectory_read(); no test previously verified the two sides agree on the field names, so a mismatch would have failed silently.Citation block format
The shared test fixtures enforce the following behavior:
Known divergence between R and Python
Explanation trimming uses each language's whitespace class, so a trailing U+00A0 survives in R but not Python. Explanation is never verified, so this is cosmetic; it's documented in the fixture.
Reviewer notes (for R)
citation_scanner()records.tests/testthat/fixtures/shared/are generated byscripts/sync-shared-fixtures.sh, so they don't need to be separately reviewed.Testing
test-citation-scan.Rpasses in full (25 tests, 217 expectations) against the rebased tree.R CMD checknot run locally (missing R deps); left to CI.