TEF-663 Raise NullFactError when a fact resolves to nil - #15
Open
dfl wants to merge 1 commit into
Open
Conversation
dfl
force-pushed
the
TEF-663-raise-on-null-fact
branch
from
July 17, 2026 03:09
a480bf2 to
fb86083
Compare
dfl
marked this pull request as ready for review
July 17, 2026 03:17
dfl
commented
Jul 17, 2026
| @@ -147,6 +147,7 @@ def call(input, results) | |||
| results[module_name] ||= {} | |||
|
|
|||
| if !resolver.respond_to?(:call) | |||
Contributor
Author
There was a problem hiding this comment.
thoughts on flipping this to unless ?
A core invariant of the fact graph is that no fact returns null: when a value can't be calculated the fact must signal an unmet dependency (an error hash), so dependent facts can tell "missing" apart from "satisfied". A fact that returns nil breaks that guarantee silently. Enforce it at evaluation time: Fact#call now raises FactGraph::NullFactError, naming the offending fact (and entity, for per-entity facts), whenever a resolver or constant resolves to nil. This covers the two ways nil leaks in — an allow_unmet_dependencies fact whose branch returns nil instead of data_errors, and an input fact whose validator permits nil. `false` remains a valid value. - lib/fact_graph.rb: add FactGraph::NullFactError - lib/fact_graph/fact.rb: ensure_non_nil! guard on both resolve paths - spec/fact_graph/fact_spec.rb: cover the raise (proc, allow_unmet, input pass-through, per-entity) and that false does not raise - README: document the no-null invariant Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dfl
force-pushed
the
TEF-663-raise-on-null-fact
branch
from
July 17, 2026 03:19
fb86083 to
768f038
Compare
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.
Ticket
https://codeforamerica.atlassian.net/browse/TEF-663
Summary
Enforces a core fact-graph invariant: no fact may resolve to
nil. When a value can't be calculated, a fact must signal an unmet dependency (an error hash /data_errors) so the facts downstream can tell a missing value apart from a satisfied one. A fact that returnsnilbreaks that guarantee silently — dependents treat the gap as answered.Fact#callnow raisesFactGraph::NullFactError— naming the offending fact (and entity, for per-entity facts) — whenever a resolver or constant resolves tonil.Why
This is a developer-error invariant, like a
NoMethodError: anilfact is a bug in the graph definition, so it should fail loudly rather than propagate. Catching it at evaluation time covers every input, everywhere the graph runs (dev, test, prod, jobs) — not just paths a spec happens to exercise. It closes the two waysnilleaks in:allow_unmet_dependenciesfact whose branch returnsnilinstead ofdata_errorsnil(the blank case should resolve to an error, not passnilthrough)falseremains a valid value and does not raise.Changes
lib/fact_graph.rb— addFactGraph::NullFactErrorlib/fact_graph/fact.rb—ensure_non_nil!guard on both the resolver and constant/pass-through pathsspec/fact_graph/fact_spec.rb— cover the raise (proc,allow_unmet_dependencies, input pass-through, per-entity message) and thatfalsedoes not raiseREADME.md— document the invariant under "Handling unmet dependencies"🤖 Generated with Claude Code