fix(identity-invariants): never echo left-of-operator text in unsupported-fact rejection - #24
Merged
Merged
Conversation
…rted-fact rejection threads-ype (N-2 from coven PR #464 fresh-context review): parse_identity_declaration echoed the left-of-operator text verbatim in the 'unsupported identity fact' rejection. A reversed declaration like 'Val Alexander' == familiar.person would carry principal-adjacent text into migration reports and stdout. The rejection now names only the expected grammar (the five supported familiar.* facts) and never echoes input. The 'unsupported identity fact' prefix is preserved — downstream assertions in the retired-Ward corpus and coven ward_migrate tests match on that prefix only. Adds a regression test proving a reversed declaration is rejected without echoing the principal value. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
BunsDev
force-pushed
the
fix/threads-ype-elide-invariant-echo
branch
from
July 27, 2026 06:17
e381494 to
dd314cc
Compare
BunsDev
marked this pull request as ready for review
July 27, 2026 06:18
There was a problem hiding this comment.
Pull request overview
This PR tightens identity-invariant parsing to avoid echoing potentially principal-adjacent input text in error messages, while preserving downstream-compatible error prefixes and adding a regression test to prevent reintroduction.
Changes:
- Replace the
unsupported identity fact …rejection so it no longer includes the left-of-operator input text, and instead reports the supportedfamiliar.*grammar. - Add a regression test ensuring reversed declarations are rejected without leaking the principal-like text.
- Preserve downstream compatibility by keeping the
unsupported identity factprefix and relying oninvariant[N]:indexing for locability.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Val Alexander <bunsthedev@gmail.com>
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.
Closes threads-ype (N-2 from the fresh-context review of OpenCoven/coven#464).
Problem
parse_identity_declarationformattedunsupported identity fact {:?}echoing the left-of-operator text verbatim. A reversed declaration like'Val Alexander' == familiar.personwould echo principal-adjacent text into migration reports and stdout.Fix
The rejection now names only the expected grammar (the five supported
familiar.*facts) and never echoes input:Downstream compatibility
The
unsupported identity factprefix is preserved. Both known downstream assertions match on that prefix only:tests/support/phase5_retired_ward_corpus.rs(expected_error_contains)covenPR #464ward_migrate.rsrejection-lane testPer-declaration locability is retained via the
invariant[N]:index prefix added byIdentityInvariantSet::compile.Validation
cargo fmt --checkandcargo clippy -p coven-threads-core --all-targetsclean