Conversation
A 490 carrying an enumeration or ISSN subfield alongside its $a came out as two identical bf:relation values on the Work -- under libxslt, but not under Saxon. The group numbers of the grouped 490 fragment sit on its top-level children, so preceding-sibling:: reaches all of them, and says what the code means more directly than walking preceding:: from an attribute. It also avoids a libxml2 bug: over a node-set made with exsl:node-set, the preceding:: axis omits the fragment's first top-level element. The group number being compared against was therefore never in the node-set, nothing was filtered, and every child of the fragment built its own bf:Relation. Saxon walks the axis correctly, which is why the XSpec suite has always passed and this only ever reached users on libxslt -- lxml, Nokogiri, PHP's XSL extension, XML::LibXSLT and xsltproc. test/data/ConvSpec-Process6-Series/dup-relations.xml and a new scenario expect one relation from each of 490 $a, $a $v and $a $x, following the dup-properties.xml pattern in ConvSpec-001-007. They pass under Saxon with or without the fix, so they guard the behaviour rather than demonstrate the bug. test/compare-processors.sh is what demonstrates it. It converts the same records with both processors and requires the results to match, after rendering both through test/normalize.xsl so that indentation, attribute order and the placement of namespace declarations are not differences. CI installs xsltproc, which the config already anticipated in a commented-out line, and runs the script after the XSpec suites. Its default record set is deliberately small so the check is green from the start; pointing it at other paths is how it widens. No change under Saxon: output is byte for byte identical over test/data/marc.xml, test/data/collection.xml and test/data/ConvSpec-Process6-Series/marc.xml, and the full XSpec suite still passes 1378/1378. Under libxslt the relation counts now match Saxon's exactly. Fixes lcnetdev#273
edsu
commented
Sep 10, 2026
| <!-- Find the group numbers. --> | ||
| <xsl:variable name="tThisDF" select="."/> | ||
| <xsl:variable name="tGroupNums" select="$grouped490Info//@groupNum[not(.=preceding::bf:*/@groupNum[1])]" /> | ||
| <xsl:variable name="tGroupNums" select="$grouped490Info/*[not(@groupNum = preceding-sibling::*/@groupNum)]/@groupNum" /> |
Author
There was a problem hiding this comment.
This is the one line fix! Everything else is just testing with xsltproc and docs.
edsu
added a commit
to blue-core-lod/bluecore-models
that referenced
this pull request
Sep 16, 2026
We were refusing to load any graph in which a resource carried the same blank node value twice, raising DuplicateValueError. That turns away about a quarter of what LC actually serves: of the 346,991 harvested CBDs in uploads/, 89,915 (25.9%) carry at least one duplicate. The cause is upstream and not ours to fix. One source at least is the marc2bibframe2 XSLT stylesheet which we have patched, but the version that LC provides still has the problem (although it's unclear that they use it internally): lcnetdev/marc2bibframe2#274 A duplicate is redundant by definition: the values are identical in content all the way down, so dropping all but one loses nothing. Detect them, log what was dropped, strip them, and load the record. - DuplicateValue gains a `redundant` field naming the copies past the first. find_duplicate_bnode_values already grouped these nodes in order to count them; it just discarded them afterwards. Detection is unchanged. - strip_duplicate_bnode_values() unlinks each redundant node and takes its description with it. A blank node that is also another resource's value keeps its description. - _check_duplicate_values becomes _strip_duplicate_values: same message, now a warning with a count of what was removed, and no raise. Still runs first in save(), judging the payload before our own normalization. - DuplicateValueError is removed, since we're no longer raising it. - _remove_bnode moves to utils.graph as remove_bnode, so the stripper and the existing callers share one recursive remover. Closes #170
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.
Full disclosure: these edits were suggested by Claude Code. Really there's just a one line change that's needed for things to work correctly under Saxon and libxslt. If you want to zoom in on that, I've highlighted it below with a comment. But if you want the additional testing bits, that's where the majority of the PR is. If you'd rather have the PR trimmed back to the bare essentials let me know.
A 490 carrying an enumeration or ISSN subfield alongside its $a came out as two identical bf:relation values on the Work -- under libxslt, but not under Saxon.
The group numbers of the grouped 490 fragment sit on its top-level children, so
preceding-sibling::reaches all of them, and says what the code means more directly than walkingpreceding::from an attribute. It also avoids a libxml2 bug: over a node-set made withexsl:node-set, thepreceding::axis omits the fragment's first top-level element. The group number being compared against was therefore never in the node-set, nothing was filtered, and every child of the fragment built its own bf:Relation. Saxon walks the axis correctly, which is why the XSpec suite has always passed and this only ever reached users on libxslt: lxml, Nokogiri, PHP's XSL extension, XML::LibXSLT and xsltproc.test/data/ConvSpec-Process6-Series/dup-relations.xmland a new scenario expect one relation from each of 490 $a, $a $v and $a $x, following the dup-properties.xmlpattern inConvSpec-001-007. They pass under Saxon with or without the fix, so they guard the behavior rather than demonstrate the bug.test/compare-processors.shis what demonstrates it. It converts the same records with both processors and requires the results to match, after rendering both throughtest/normalize.xslso that indentation, attribute order and the placement of namespace declarations are not differences. CI installs xsltproc, which the config already anticipated in a commented-out line, and runs the script after the XSpec suites. Its default record set is deliberately small so the check is green from the start; pointing it at other paths is how it widens.No change under Saxon: output is byte for byte identical over
test/data/marc.xml,test/data/collection.xmlandtest/data/ConvSpec-Process6-Series/marc.xml, and the full XSpec suite still passes 1378/1378. Under libxslt the relation counts now match Saxon's exactly.Fixes #273