Fix the two temporal defects the campaign scale run found - #33
Merged
Conversation
Running the temporal pass over 40 real instruments instead of a handmade fixture broke invariant 6 nine times, and none of the nine was what the check is for. The first defect is in the check. A repeal terminates more versions than it repeals: when a clause goes, every component above it is re-issued without that child, and the old version of each of them is terminated by the same repeal event. Read naively that is an article repealed and brought back the same day. Now the terminating event has to have targeted the component itself or something above it, which takes the nine down to three. The remaining three were a real bug in the builder. depthOrder took depth from the number of colons in the identifier, but an article is numbered against its instrument and not against the chapter holding it, so a two segment section counted as deeper than the one segment articles inside it. The section got its version first, its children had no versions yet, and it came out holding empty strings: 210 dangling child links across 33 components corpus wide, a consolidated chapter text with nothing in it, and parent links broken in exactly the way invariant 6 reads as resurrection. Depth now comes from walking the parent chain. The real layer went from nine violations to none, and from 210 empty child identifiers to none, with the same 2031 versions of 1971 components from the same 52 events. Refs #18
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.
M14 was about running the passes at breadth to find out what breaks, and the temporal pass over 40 real instruments broke invariant 6 nine times. None of the nine was a provision coming back from the dead, which is the only thing that check exists for.
The check was wrong
A repeal terminates more versions than it repeals. When one clause goes, every component above it gets a new version without that child, and the old version of each of them is terminated by the same repeal event. Read naively, an article that lost a clause looks like an article that was repealed and came back the same day. The corpus says so out loud: Quyết định 06/2001 had provisions repealed out of two of its articles by two later decisions, and the article that lost a clause of Nghị định 03/2003 did the same thing.
So the terminating event now has to have targeted the component itself or something above it. A repeal aimed at a descendant is a component being re-issued smaller and that is ordinary. Nine violations became three.
The first attempt at that test was a string prefix on the identifier and it only got as far as six. It missed the chapter and section case, for the same reason the second bug exists.
The builder was wrong
depthOrdertook depth from the number of colons in the identifier. An article is numbered against its instrument and not against the chapter holding it, so Điều 9 of a decree is...:article-9whichever chapter it sits in, while the chapter and section around it are two segments deep. Counting segments made the container look deeper than its contents, so the section got its version written before the articles under it existed and its children came out as empty strings.Nothing failed loudly. Corpus wide it was 210 dangling child links across 33 components, a chapter whose consolidated text assembles to nothing, and parent links broken in exactly the way invariant 6 reads as resurrection. Depth now comes from walking the parent chain, which is the thing that actually states the structure.
What the real layer says now
Same 2031 versions of 1971 components from the same 52 events, and:
Tests
TestAnArticleThatLostAClauseWasNotItselfRepealedcovers the check, andTestCheckCatchesLifeAfterRepealstill passes, so a genuine resurrection is still caught.TestAChapterHoldsTheArticlesUnderItbuilds a document shaped the way most decrees are, a chapter holding a section holding articles, and asserts no version holds an empty child identifier and that an amendment at the bottom shows up in the chapter's text. Against the old code it fails with four empty identifiers and an empty chapter text.Gate is clean: gofmt, go vet, golangci-lint 0 issues, and
go test -race -covergreen across all 27 packages.Refs #18