Repository navigation
Implement English structural profile with complete-corpus replay - #81
erinepshovel-code wants to merge 14 commits into
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82625fd091
ℹ️ 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".
| "tree_sha256": snapshot.source_tree_sha256, | ||
| "files": snapshot.source_file_count, | ||
| "lexemes": len(snapshot.lexemes), "synsets": len(snapshot.synsets)}, | ||
| "complete_corpus": complete_corpus, "producer": producer, |
There was a problem hiding this comment.
complete_corpus is copied directly from the caller even though verify() only reconciles rows against the artifact’s embedded evidence and never checks the expected OEWN tree digest or corpus counts. A caller can therefore pass a one-lexeme WordnetSnapshot, the valid carrier metadata, and complete_corpus=True; the resulting artifact verifies successfully while claiming the pinned OEWN commit and complete-corpus standing. Derive this flag only after run() validates the source checkout, or make verification enforce the exact pinned digest and counts, so truncated or substituted corpora cannot pass the advertised complete-corpus gate.
AGENTS.md reference: AGENTS.md:L11-L12
Useful? React with 👍 / 👎.
| substantive = [p for p in graph["parts"] if "separator" not in p["function"]["roles"]] | ||
| closed = [c for c in graph["closure"] if c["state"] == "closed"] | ||
| if (not substantive or len(closed) != 1 or closed[0]["part"] != substantive[-1]["ordinal"] | ||
| or any(c["state"] == "hmmm" for c in graph["closure"])): |
There was a problem hiding this comment.
For quoted or bracketed sentences such as "geometry.", segment() deliberately marks the period as closed because it is followed by a closing quote, but this condition then requires that period to be the final non-separator part. The trailing quote is classified as punctuation rather than a separator, so compose(..., relation="sentence") returns SENTENCE_CLOSURE_UNRESOLVED for the very closing-punctuation case the segmenter recognizes. Account for permitted trailing closing punctuation when locating the terminal closure.
Useful? React with 👍 / 👎.
| p = meta["profile"] | ||
| if p != policy(p["abbreviations"]) or meta["profile_sha256"] != digest(p): | ||
| raise StructuralError("profile mismatch") | ||
| if meta["implementation_sha256"] != _implementation(): | ||
| raise StructuralError("implementation mismatch; explicit migration required") |
There was a problem hiding this comment.
verify() validates the profile and implementation digest but never enforces the fixed authority_transfer or geometry_state metadata. If either metadata row is changed and the manifest is resealed with the recomputed row receipt, verification succeeds with values such as authority_transfer=true and geometry_state="executed", allowing an artifact to claim a transfer of authority or resolved geometry that this component explicitly cannot confer. Validate the complete fixed metadata envelope, including these boundary fields, before returning the manifest.
AGENTS.md reference: AGENTS.md:L13-L15
Useful? React with 👍 / 👎.
| cmp _state/first/construct.db _state/second/construct.db | ||
| cmp _state/first/manifest.json _state/second/manifest.json | ||
| python -m english_gonol.structural_query _state/first 'Dr. creates geometry. Dr. creates 3.14...' > _state/query.json |
There was a problem hiding this comment.
The complete-corpus workflow compares the two newly generated databases and manifests only with each other, then generates a new query without comparing either result to the tracked experiments/structural-profile-v1/manifest.json or query.json. A behavior-bearing change can therefore pass CI while those repository-owned receipts still reference the old implementation digest, field receipt, and query result, leaving the documented profile evidence stale. Add a comparison against the committed evidence or an explicit evidence-update gate in this workflow.
AGENTS.md reference: AGENTS.md:L86-L89
Useful? React with 👍 / 👎.
… main Reviewed source tree matches local 12dbb5cf63277b544d144830e9248d729b3ce179. Research branch only; no graduation or authority transfer.
Research branch only. Exact source and clean-wheel full corpus rebuilds agree; committed evidence and extraction gates are enforced. No graduation or authority transfer.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfd62f6ff9
ℹ️ 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".
| if (not terminal or any(c["state"] != "closed" or c["part"] not in terminal | ||
| for c in graph["closure"])): |
There was a problem hiding this comment.
When a nonterminal ambiguity is explicitly resolved with closed: false, the closure list contains an open entry plus the final closed mark. This condition rejects every state other than closed, so a correctly annotated sentence such as Dr. creates geometry. still returns SENTENCE_CLOSURE_UNRESOLVED. An open decision is a resolved non-boundary and should not block the terminal closure, while hmmm and nonterminal closed entries still should.
Useful? React with 👍 / 👎.
| elif i > 0 and (i + 1 == len(text) or text[i + 1].isspace() or text[i + 1] in '\"”’)]}'): | ||
| roles, closure = ("sentence-close",), "closed" |
There was a problem hiding this comment.
For admitted text such as 'geometry.', the period remains hmmm because this closing-scalar allowlist contains the curly apostrophe but omits the ordinary ASCII single quote; the matching allowlist in compose() omits it as well. Fresh evidence in this revision is the explicit new allowlist in both stages, which fixes double and curly quotes but still makes this common quoted-sentence form return SENTENCE_CLOSURE_UNRESOLVED.
Useful? React with 👍 / 👎.
| assert hashlib.sha256(json.dumps(identity, sort_keys=True, separators=(',', ':')).encode()).hexdigest() == graph['work_graph_sha256'] | ||
| source = next(r['commit'] for r in graph['repositories'] if r['repository'] == 'The-Interdependency/stack') | ||
| assert subprocess.check_output(['git', '-C', '_deps/forge', 'rev-parse', 'HEAD'], text=True).strip() == source | ||
| files = ['english_gonol/structural_rules.py', 'english_gonol/structural_construct.py', 'english_gonol/structural_query.py', 'tests/test_structural_profile.py'] |
There was a problem hiding this comment.
This immutable-forge comparison covers only the three structural modules and the test file, while _implementation() in structural_construct.py also hashes hyperspace_construct.py, full_construct_run.py, and language/source.py. If any of those behavior-bearing dependencies changes and the committed manifest/query are regenerated, this workflow can pass even though the runtime implementation no longer matches the work graph's pinned forge commit; compare every input to the implementation digest against the pinned checkout.
AGENTS.md reference: AGENTS.md:L72-L79
Useful? React with 👍 / 👎.
| self.manifest = verify(out_dir) | ||
| self.db = open_readonly(Path(out_dir) / "construct.db") | ||
| self.words = MappingProxyType(dict(self.db.execute("SELECT surface,id FROM words"))) | ||
| self.characters = MappingProxyType({s: (cid, pos) for cid, s, pos in self.db.execute("SELECT * FROM characters")}) | ||
| self.abbreviations = self.manifest["metadata"]["profile"]["abbreviations"] |
There was a problem hiding this comment.
manifest remains publicly mutable, and abbreviations aliases its mutable profile list, while construct() later reads both receipt hashes and parser policy from those objects. After initialization, a caller can replace manifest['receipt_sha256'], alter profile_sha256, or remove Dr from the abbreviation list; subsequent construction changes its closure behavior, emits the forged identities, and replay() still accepts the record. Snapshot these verified values into immutable internal state so caller mutation cannot redefine the field after verification.
Useful? React with 👍 / 👎.
Preserve explicit open non-boundaries, admit ASCII closing quotes, validate complete SQLite schema, and snapshot verified receipt policy. Includes 67 structural regression tests; research-only source checkpoint for exact UCHC extraction.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e27f9e54d5
ℹ️ 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".
| self._field_receipt = verified["receipt_sha256"] | ||
| self._profile_sha256 = verified["metadata"]["profile_sha256"] | ||
| self._abbreviations = tuple(verified["metadata"]["profile"]["abbreviations"]) | ||
| self.db = open_readonly(Path(out_dir) / "construct.db") |
There was a problem hiding this comment.
When the artifact directory remains writable, verify(out_dir) closes its database connection before this line opens the file again, and later word() and definition() calls continue reading that independently opened file. A concurrent writer can replace or modify construct.db between verification and opening—or after initialization—so the field returns unverified rows while retaining the original receipt; for example, changing word_characters after constructing a StructuralField immediately changes field.word(...). Verify and query through the same connection and stable read transaction, or otherwise bind the opened bytes to the verified receipt.
Useful? React with 👍 / 👎.
| name: English structural profile | ||
| on: | ||
| pull_request: | ||
| paths: ['research/english-gonol/**', '.github/workflows/english-structural-profile.yml'] |
There was a problem hiding this comment.
The workflow later reads research/zfae/UCHC_INPUT.json and asserts that it matches MIGRATION.json, but both path filters exclude that lock. A PR that advances the historical UCHC input therefore runs uchc-input.yml while skipping this cross-projection check, allowing the English migration record and ZFAE lock to drift unnoticed; include the lock in both the pull-request and push filters.
AGENTS.md reference: AGENTS.md:L72-L79
Useful? React with 👍 / 👎.
…raction candidate
1 participant
The previous non-whitespace token rule admitted punctuation into word identity and could not express ambiguous sentence closure independently. This implements the accepted English structural profile in the Stack forge.
Validation: 150 English tests and 5 Python-domain tests passed; 20 structural tests also passed from the clean-installed candidate. Two independent complete corpus constructions (installed development wheel with the same implementation digest and forge source) produced byte-identical 650,153,984-byte databases and manifests. Native module projection: 4 modules, zero attachment findings; 4 ratio seals verified; Stack consistency passed.
Evidence: research/english-gonol/experiments/structural-profile-v1/. The new corpus contains 103,053 word identities and preserves all 185,155 definition records.
Paired UCHC extraction: The-Interdependency/uchc#9. Stack retains forge authority; candidate extraction does not confer graduation or a stable release.
hmmm: 57,155 compound/punctuated lexical definitions retain unresolved word-origin attachment. UCNS geometric order/displacement remains unresolved and refused. Parser conventions beyond the accepted structural answers are explicit candidate choices.