Repository navigation
feat(weave): construct eight-to-two positional degeneration candidate - #73
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa48511814
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| identities = tuple(circle.identity for circle in self.circles) | ||
| if len(set(identities)) != 8: | ||
| raise ValueError("circle identities must be unique") |
There was a problem hiding this comment.
Reject unrelated circles as a derived key set
When callers provide eight arbitrary Circle objects—as the new test fixture does—this constructor accepts them solely because their labels differ, even though nothing identifies one as the whole circle or proves that the other seven derive from it. Consequently, the implementation and receipt report an eight-circle construction while testing only tuple cardinality; because the derivation law is unresolved, this boundary should remain hmmm or require explicit derivation provenance rather than accepting unrelated circles.
AGENTS.md reference: AGENTS.md:L22-L26
Useful? React with 👍 / 👎.
| if type(self.circles) is not tuple or len(self.circles) != 2: | ||
| raise ValueError("public degeneration exposes exactly two circles") | ||
| if any(type(circle) is not Circle for circle in self.circles): | ||
| raise TypeError("public circles must be Circle instances") |
There was a problem hiding this comment.
Require two distinct circles in PublicPair
When PublicPair is instantiated directly, PublicPair((0, 1), (circle, circle)) passes validation and is accepted by encode_byte, so a single circle can masquerade as the promised two-circle public degeneration. Validate distinct circle identities (or prevent direct construction) in addition to distinct indices so the public-pair invariant holds for every accepted encoder input.
Useful? React with 👍 / 👎.
| def position(self, bit: int, origin: Fraction) -> Fraction: | ||
| """Exact key-set position relative to this circle's space attachment.""" | ||
| _bit(bit) | ||
| return _phase((self.one if bit else self.zero) - self.space + _phase(origin)) |
There was a problem hiding this comment.
Leave the circle-space relation unresolved
When the intended UCHC circle/space relation is anything other than an additive scalar offset, this formula silently replaces it with bit_position - space + origin (mod 2), even though EIGHT_CIRCLE.md says no rule for those relations has been supplied and the existing Weave specification leaves native origin/axis binding open. The resulting round-trip and attack evidence therefore measures an invented geometry; this operation should remain hmmm or be supplied explicitly by a source-backed construction instance.
AGENTS.md reference: AGENTS.md:L22-L26
Useful? React with 👍 / 👎.
Implements the current Weave construction without inventing additional key-selection laws.
Construction
Evidence
Adds six focused tests for eight-member/full vs two-member/public boundaries, exact 720 return, variable per-key-set space relations, 2x expansion and roundtrip, malformed witness refusal, and a preserved attack witness.
Falsification
The first independent-bit two-circle projection is publicly brute-forceable: the public holder can compare the two public forward images for 0 and 1. This baseline is retained as a failing cryptographic idea, not promoted to Weave security.
The geometry survives; the next construction must use the message-scoped UCHC origin/axis relational state so the six omitted circles are causally required for inversion.
No existing transport behavior, UCNS authority, or UCHC authority is changed.
hmmm: the coupled UCHC eight-circle relation remains the live construction boundary.