fix(contribution): the unobserved cap was built and never called - #26
Open
isaacismaelx14 wants to merge 1 commit into
Open
fix(contribution): the unobserved cap was built and never called#26isaacismaelx14 wants to merge 1 commit into
isaacismaelx14 wants to merge 1 commit into
Conversation
A04 shipped `capUnobserved` with full domain tests, declared UNOBSERVED_POINTS_PER_DAY and UNOBSERVED_POINTS_PER_MONTH in the environment schema, and then nothing read either one. ContributionService awarded `pointsFor(...)` straight to the ledger, so 24's cap existed in the codebase and not in the running system. That is worse than not having built it, because everything pointed the other way: the domain tests pass, the configuration is documented, and the reviewer sees a cap. Only the award path knew there wasn't one. An observer is decided with `isStructurallyIndependentReviewer`, the same predicate eligibility already uses, rather than a second one that could drift from it. That covers three of 24's four clauses. The fourth, that the observer is not an account the contributor controls, is a Risk Engine judgement Core holds no evidence for, and it is asserted here for the reason the domain already gives for eligibility: the predicate is necessary and never sufficient, Law XXVIII keeps recognition's bar below issuance's, and the Network re-decides anything touching money. The comment says so rather than leaving it to be inferred. The tally needs somewhere to count from, so contribution_entries gains a nullable `observed`. Null means not evaluated, not "nobody watched": every row written before A04 predates the question and Law XV does not permit history to be restated, so they stay out of the count rather than being backfilled with a guess. Invalidations net out, or a reverted PR would keep somebody capped for work the economy already withdrew. The allowance is measured against the day the work happened, not the day Kreds looked. Delegated query makes backfill ordinary, and counting against today would let one sweep spend an allowance belonging to a different day. One consequence worth stating plainly rather than leaving to be found: an instance with no configured allowance now awards zero points for any merge no independent human reviewed. That is Law XIX applied to a missing setting, and it is what the tests assert.
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.
A04 shipped
capUnobservedwith full domain tests, declared the two environment variables, and then nothing read either one.ContributionServiceawardedpointsFor(...)straight to the ledger.So chapter 24's cap existed in the codebase and not in the running system. Worse than not having built it: the domain tests pass, the configuration is documented, and a reviewer sees a cap. Only the award path knew there wasn't one.
Found while verifying that the production variables had been set correctly, which is the only reason it surfaced at all.
What decides an observer
isStructurallyIndependentReviewer, the same predicate eligibility already uses, rather than a second one that could drift from it. That covers three of 24's four clauses.The fourth, that the observer is not an account the contributor controls, is a Risk Engine judgement Core holds no evidence for. It is asserted here for the reason the domain already gives for eligibility: necessary and never sufficient, Law XXVIII keeps recognition's bar below issuance's, and the Network re-decides anything touching money. The comment says that rather than leaving it to be inferred.
Where the tally comes from
contribution_entriesgains a nullableobserved. Null means not evaluated, not "nobody watched": rows written before A04 predate the question, and Law XV does not permit history to be restated, so they stay out of the count rather than being backfilled with a guess.Invalidations net out, or a reverted PR would keep somebody capped for work the economy already withdrew.
The allowance is measured against the day the work happened, not the day Kreds looked. Delegated query makes backfill ordinary, and counting against today would let one sweep spend an allowance belonging to a different day.
One consequence, stated rather than left to be found
An instance with no configured allowance now awards zero points for any merge no independent human reviewed, public or private. That is Law XIX applied to a missing setting, and the tests assert it.
It also means the strict reading of "observer" as a person has teeth for solo public maintainers. Raised separately: it may be a question for the law rather than for this repository.
Verified by mutation
Skipping the cap, accepting a self-review as an observer, measuring the tally from now, and marking everything observed each turn tests red.