Optimize the diff algorithm, part 4 - #6
Conversation
There was a problem hiding this comment.
Pull request overview
Improves token attribution when identical text appears in competing structural locations, with optional native acceleration.
Changes:
- Adds candidate-ledger resolution and structure-aware duplicate matching.
- Introduces native acceleration with Python fallbacks.
- Adds authentic lineage fixtures and differential regression tests.
Reviewed changes
Copilot reviewed 5 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| WikiWho/wikiwho.py | Implements structural matching and candidate resolution. |
| WikiWho/utils.py | Fixes table parsing and occurrence traversal. |
| WikiWho/_structural_native.c | Adds native structural-matching kernels. |
| setup.py | Configures the optional extension. |
| tests/test_structural_native.py | Compares native and Python behavior. |
| tests/test_regression.py | Expands matcher regression coverage. |
| tests/authorship_cases.json | Adds authentic attribution cases. |
| .gitignore | Ignores macOS metadata. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
# Conflicts: # tests/test_regression.py
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (2)
tests/authorship_cases.json:152
- This regression only checks the
2012token, so an incorrect transfer ofSeptemberfrom the removedunreftemplate would still pass. The stated Bradshaw boundary requires the completeSeptember 2012value to retain revision510854404; include both tokens in the assertion.
"focus": ["2012"],
"expected": {
"values": ["2012"],
"origin_rev_ids": [510854404]
}
WikiWho/utils.py:47
- The comment describes
|}}as the matched template ending, but the negative lookahead deliberately excludes that sequence and matches only line-start|}table closers. Update it so future changes do not invert this parsing rule.
# Treat a line-start `|}}` as a template ending. It is ambiguous with a
# table close followed by a literal `}`, which this fast path cannot parse.
Review notes — analysis run with Claude CodeHow to read this. These findings come from an AI-assisted review session. They are not to Scope. Reviewed as One method note for anyone re-running this. The extension's The change decomposes cleanly, and that is the most useful thing here
With that one call skipped, this branch is byte-identical to #5 on all twelve fixtures — So restructuring every existing matching stage to submit proposals to a ledger is verifiably Verified claimsNative and Python agree exactly. This is the branch's central correctness claim and it holds
The C range table in T434097 is genuinely fixed, and the boundary holds. Running this branch's The C is careful. Every size multiplication is guarded (
Attribution impact is small. Over 30,327 revisions and 16 articles in 11 wikis, aligning the Concerns1. The pure-Python fallback is much slower than #5, and a build failure is silent. Best of three, same fixtures, same interpreter:
The "+4.3% runtime" in the description therefore appears to describe the native path against That matters operationally because of how the two interact: 2. Origin changes outside the targeted class adjudicate as a coin flip. Of the 267 origin differences, 84 were decidable by introduced-vs-inherited — does the credited
Per article: en/83780967 30–28 to #6, en/349784 3–1 to #6, ar/4287 2–1 to #6, hi/59 3–6 to #5, 3. 15% of aligned tokens (42,265) get a different A kill switch looks cheap and worth havingThe structural layer enters through exactly one call before
Given this is a ~7,500-line change to production attribution with no CI, being able to disable The gate used for these measurements was written by Claude Code and has not been run or Smaller notes
What this review did not coverThe 3,989 lines of C were not audited line-by-line for reference-count correctness; valgrind Drafted in a Claude Code session (~1 h on this review, within a longer session of ~2.5 h and 9 (Comment written by Claude Code.) |
|
I agree with Claude Code that the existing cases do not demonstrate the improvements yet. Therefore I'm adding ten more test cases. |
1. Yoshiminosato Station: URL moved out of repeated citationsTarget: | website = {{Official website|1=http://www.nankai.co.jp/traffic/station/yoshiminosato}}Revision sequence:
Under WikiWho’s token-reuse semantics, those tokens came from 2. 天草市立二浦小学校: duplicate CJK characters in a newly added nameTarget—the second occurrence of: 早浦亀浦両村組合立 白石小学校The test specifically selects the first Revision sequence:
Because WikiWho tokenizes this Japanese text character by character, the old matchers borrow the matching 3–5. Jewish Sports Review: unchanged citation fields reset by a dash editThe three targets come from the same citation: |title=Jewish sports legends: the International Jewish Hall of Fame
|publisher=Books.google.com
|date=
|accessdate=May 12, 2010They test three kinds of tokens:
Revision sequence:
The selected publisher, hostname, and date-field tokens do not change in These are three tests because they cover a template field key, a field value, and neighboring empty/nonempty field keys. 6. Richard Thompson: moved award name is wrongly treated as newly writtenTarget text: Marsh Award for Marine and Freshwater ConservationRevision sequence:
The seven words themselves are unchanged. Duncan.Hull authored the restructuring and unlinking, but not those words. Correct attribution on Both Why this is adjudicable:
7. Meeting at Night: retained phrase is reset during a lead rewriteTarget text, including quotation marks: "Night" and "Morning"It tokenizes into seven tokens: Revision sequence:
Correct attribution on Both Why this is adjudicable:
8. Johannes Westö: a new infobox field steals
|
The matcher introduced in #5 improves several attribution cases, but it still cannot reliably preserve token identity when identical text occurs in multiple structural locations, as raised by @ragesoss in https://phabricator.wikimedia.org/T434097 on the Yeah Yeah Yeahs (EP) article. The failure mode is :
1625540(2003).748288963instead of retaining token IDs5,6, and7from revision1625540.This is an occurrence-assignment problem. The existing matcher operates over flattened article-wide word arrays. Its matching stages—edge matching,
SequenceMatcher, positional fallback, moved-run recovery, and construct-boundary recovery—assign token identities as they execute. When two identical occurrences compete, an early stage may consume the wrong previous token identity. Later evidence cannot reliably distinguish:The matcher therefore needs to consider competing evidence before finalizing token reuse, and it needs paragraph and sentence occurrence information to distinguish structurally separate copies.
A second authentic case, Bradshaw, West Virginia, defines an important boundary for the fix. Two different templates contain the textual value
September 2012. When theunreftemplate is removed, the remainingbiasvalue must retain its own origin, revision510854404, rather than inherit the olderunreftoken from revision510852310. Identical surrounding text must not override distinct structural ownership.Therefore this #6 extends from #5 with the following matcher algorithm changes:
Proposed fixes and major algorithm changes
1. Separate match discovery from final token assignment
All word-matching stages now submit proposals to a central candidate ledger.
Each candidate records:
The ledger maintains a provisional compatibility view for candidate generators that depend on earlier matches, but these provisional assignments do not determine the final result.
After all applicable evidence has been collected, one resolver produces the final one-to-one token mapping.
The evidence order is explicit:
SequenceMatchermatches;Structurally supported matches therefore outrank generic article-wide sequence alignment, while verified globally unique moved text can still outrank a merely local structural correspondence.
2. Preserve explicit structural occurrence identity
The structural matcher uses token slots carrying:
Wordobject when one already exists.Paragraph and sentence hashes are not treated as unique occurrence identifiers because identical paragraphs and sentences may legitimately appear multiple times.
The previous revision is read from its persistent hierarchy. The current revision reuses the hierarchy already produced during parsing. If the hierarchy is incomplete, inconsistent, or aliases paragraph, sentence, or word objects in a way that prevents reliable occurrence identification, structural matching fails closed to the original tokenizer-based representation.
3. Gate structural analysis on actual duplicate competition
Structural matching is not run for every edited revision.
A conservative residual preflight first checks whether the unresolved portions of both revisions share a potentially duplicated window. If they do not, the matcher retains the existing result without constructing full structural context.
Only revisions with a genuine unresolved duplicate competition proceed to structural anchor discovery.
This keeps the new behavior targeted at the class of problems it is designed to solve.
4. Establish structural correspondence with globally unique anchors
For relevant revisions, the matcher indexes informative windows of 10, 8, 6, and 4 tokens.
An anchor is usable only when its exact contextual key occurs once in the complete previous revision and once in the complete current revision. These globally unique anchors associate paragraph occurrences without relying on paragraph hashes.
Compatible overlapping anchors are merged into longer segments. Paragraph pairs are accepted only when they have sufficient lexical support and are uniquely preferred from both directions.
Paragraph starts and ends become virtual anchors only for unambiguous one-to-one paragraph correspondences. Paragraph splits and merges do not receive broad virtual boundaries merely because one correspondence is larger than another.
This is what separates the lead occurrence of
Yeah Yeah Yeahsfrom the identical infobox occurrence.5. Align only the gaps between certified anchors
Once a paragraph correspondence has been established, the matcher runs a bounded exact LCS inside the gaps between its anchors.
The alignment score applies deterministic priorities:
The left-to-right rule preserves established behavior for cases such as
said saidbecomingsaid, where text alone cannot determine which historical occurrence survived.Gap alignment is capped at 50,000 cells. Larger or insufficiently supported gaps do not receive structural matches.
6. Require duplicate and boundary evidence for structural runs
A run found inside a certified gap is not automatically accepted. Therefore, The matcher verifies that:
These checks prevent textual continuity from transferring identity across unrelated templates, as in the Bradshaw case.
7. Resolve structural conflicts as complete runs
Structural candidates are accepted atomically rather than as independent token edges.
A structural candidate is rejected if any part conflicts with stronger evidence or an already selected structural run. This prevents the resolver from constructing a mixed mapping that no candidate generator actually proposed.
Structural and lower-tier candidates are grouped into connected conflict components through their current and previous token endpoints. Components larger than 512 endpoints are considered too broad to disambiguate safely and retain the established mapping.
The final structural result is also rejected if it:
This keeps the structural layer conservative outside well-supported duplicate-lineage cases.
8. Add compact and targeted structural indexing
The production path avoids allocating a full structural object for every complete-revision token.
It instead builds compact documents containing:
Anchor discovery begins with keys from residual-bearing paragraphs and then verifies those exact keys over both complete revisions. When the targeted set becomes too broad, the matcher uses the complete exact scan instead.
Only one anchor width is retained at a time. Exact multi-pattern automata are used above measured document-size thresholds, with a 250,000-symbol limit to prevent unusually broad edits from creating an excessive transient object graph.
These routing choices affect cost only. Tuple scanning, automaton scanning, and the original slot-based fallback produce the same occurrence states and candidate evidence.
9. Add optional native acceleration for exact hot loops
An optional CPython extension accelerates the most frequently executed exact operations:
The native extension does not decide token lineage. Candidate generation, evidence ordering, and conflict resolution remain in Python.
Every native kernel has an exact Python implementation. The extension is optional on Python 3, so a compilation or import failure falls back to the tested Python path rather than preventing WikiWho from being installed.
The native implementation changes constant factors—principally Python object creation, attribute access, and interpreter dispatch—without changing the matcher’s asymptotic bounds or correctness rules.
Minor notable changes
iter_rev_tokens()now uses occurrence counters scoped separately to the revision’s paragraph level and each paragraph’s sentence level. This correctly traverses repeated paragraph and sentence hashes without repeated list counting.setup.pyand uses the same token-symbol alphabet as the Python tokenizer.tests/test_structural_native.pydirectly compares every native operation with its Python oracle.Yeah Yeah Yeahs (EP), covering the complete 359-revision history;Bradshaw, West Virginia, covering the structurally distinct template values.5,6, and7and origin revision1625540:510854404, ensuring that the structural matcher does not transfer identity from the removedunreftemplate.Test results
All Python tests used the canonical Python 3.9 environment.
Correctness
The complete 359-revision Yeah Yeah Yeahs (EP) history retains token IDs
5,6, and7, originating in revision1625540, through the article restructuring, removal of the infobox duplicate, and latest cached revision.Native/Python differential validation
Runtime and memory
The full Google Play run processed approximately 4,000 fixture records, accepted approximately 3,700 revisions, and produced approximately 135,000 token objects.
Traced-memory comparisons are approximate because the reference values aggregate repeated runs, while the current values are primary-run peaks. A separate process-level audit found that native acceleration reduced runtime by approximately 31% while increasing maximum RSS by approximately 1.9%, with no indication of a large hidden native allocation.
This #6 should be merged after #5, as the former is an extension of the latter.
(Since both of our maintainers mainly use LLMs to review the code now, I have this write-up, composed with help from GPT-5.6 Sol Ultra, for your LLM agents to quickly get the idea.)
Note:
After merging #5 into main, there expect to have merge conflicts, as #5 has one more commit made since #6's divergence. Still, the conflicts are rather easy to resolve.Merge conflicts are resolved.