Conversation
Aaron Otto-160: "maybe for sql and the other exernal languages and flavor/dialects we could use f# parser combinators, if that does not fit good then antlr, backlog" Binding ordering for every external-language compatibility surface Zeta ships (SQL / GQL / Cypher / Gremlin / SPARQL / Datalog / PostgreSQL wire / MySQL wire / Esper EPL / Flink SQL / Flux / PromQL / KQL etc): 1. Try FParsec (Stephan Tolksdorf's parser-combinator library). Idiomatic F#; integrates with DU-based AST; no code-gen; no Java build dependency; BSD license. 2. Fall back to ANTLR4 ONLY if FParsec demonstrably does not scale or fit: - very large ambiguous grammars (full SQL-92 + vendor) - performance-critical parsers on multi-MB queries - deeply interlocking precedence - left-recursive grammars needing transforms Hybrid allowed across surfaces (Cypher=ANTLR + signal=FParsec OK), forbidden within one surface. Per-surface written justification required in design doc. License + package notes captured: both are factory-compatible; ANTLR carries a Java build-time dependency (antlr4-tool.jar) that FParsec avoids — this is the main ergonomic cost when choosing ANTLR. Priority P1 post-v1-roadmap. Effort: S per surface justification. Composes with the F# DSL row cluster (per-entry-point DSLs + container DSL + LINQ-compatible entry points, currently in PR #334 pending merge) and the pluggable wire-protocol row below (natural first application). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
AceHack
deleted the
backlog/otto-160-parser-tech-fparsec-first-antlr-fallback
branch
There was a problem hiding this comment.
Pull request overview
Adds a new BACKLOG entry capturing the Otto-160 directive that Zeta should prefer F# parser combinators (FParsec) for external-language/dialect surfaces, with ANTLR as a documented fallback.
Changes:
- Adds a P1 BACKLOG row defining “FParsec first, ANTLR fallback” for external-language parser technology selection.
- Documents per-surface decision/justification expectations (and forbids mixing parser stacks within a single DSL surface).
Comment on lines
+809
to
+815
| `Antlr4.Runtime.Standard` + a build-time `antlr4- | ||
| tool.jar` (the code-generator). The build-time Java | ||
| dependency of ANTLR is the key ergonomic cost — it | ||
| means `tools/setup/` must ensure a JDK is present, | ||
| and `Directory.Build.props` needs a custom target | ||
| that invokes antlr4-tool at pre-build. FParsec has | ||
| none of this cost — it is pure F#. |
There was a problem hiding this comment.
The inline code span for antlr4-tool.jar is split across two lines (antlr4- on one line, tool.jar on the next). In rendered Markdown this inserts whitespace/newline into the code span, so it won’t display as the intended filename. Keep the filename in a single inline code span on one line (or drop the backticks if it must wrap).
Suggested change
| `Antlr4.Runtime.Standard` + a build-time `antlr4- | |
| tool.jar` (the code-generator). The build-time Java | |
| dependency of ANTLR is the key ergonomic cost — it | |
| means `tools/setup/` must ensure a JDK is present, | |
| and `Directory.Build.props` needs a custom target | |
| that invokes antlr4-tool at pre-build. FParsec has | |
| none of this cost — it is pure F#. | |
| `Antlr4.Runtime.Standard` + a build-time `antlr4-tool.jar` | |
| (the code-generator). The build-time Java dependency | |
| of ANTLR is the key ergonomic cost — it means | |
| `tools/setup/` must ensure a JDK is present, and | |
| `Directory.Build.props` needs a custom target that | |
| invokes antlr4-tool at pre-build. FParsec has none | |
| of this cost — it is pure F#. |
This was referenced Apr 24, 2026
AceHack
added a commit
that referenced
this pull request
Apr 24, 2026
Addresses Amara 18th-ferry correction #6: PLV = 1 can mean anti-phase locking, not same-time synchronization. Downstream detectors that rely on "PLV = 1 => synchronized" misread anti-phase coordinators as same-time coordinators. Two new functions in `TemporalCoordinationDetection`: - `meanPhaseOffset phasesA phasesB : double option` Returns the argument (angle) of the mean complex phase- difference vector whose magnitude is the PLV. Returns None when series are empty, mismatched-length, or when the mean vector has effectively zero magnitude (1e-12 floor) — in which case direction is mathematically undefined. - `phaseLockingWithOffset phasesA phasesB : struct (double * double) option` Returns both magnitude and offset in one sequence pass. Zero-magnitude case: magnitude near 0, offset = nan; near-zero magnitude is the caller's reliable "offset is undefined" signal. Existing `phaseLockingValue` contract unchanged; new primitives are additive. Downstream `Graph.coordinationRiskScore*` and any other detector consuming PLV can now add a separate offset- based term instead of collapsing both into one scalar (Amara's explicit recommendation in correction #6). 8 new xUnit tests covering: - Identical series (offset = 0) - Constant pi/4 offset (observed = -pi/4, a-minus-b convention) - Anti-phase series (magnitude 1, offset = pi) — the correction #6 regression test, contrasted against in-phase (offset 0) with identical magnitude - Uniformly-distributed differences (zero-magnitude => None) - Empty / mismatched-length / single-element edge cases - phaseLockingWithOffset magnitude matches phaseLockingValue (consistency property preventing silent detector divergence) - phaseLockingWithOffset zero-magnitude returns (near-zero, nan) - phaseLockingWithOffset returns None on empty/mismatched All 37 TemporalCoordinationDetection tests pass locally. 0 Warnings / 0 Errors build. 6th of the 10 18th-ferry corrections operationalized this week (after test-classification doc in #339, parser-tech in #338). Remaining: Wilson CIs in CartelToy tests (needs #323 landed), MAD=0 percentile-rank fallback (needs #333 landed), conductance-sign doc (needs #331 landed), artifact-output layout (Stage-2 with calibration harness). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AceHack
added a commit
that referenced
this pull request
Apr 24, 2026
Addresses Amara 18th-ferry correction #6: PLV = 1 can mean anti-phase locking, not same-time synchronization. Downstream detectors that rely on "PLV = 1 => synchronized" misread anti-phase coordinators as same-time coordinators. Two new functions in `TemporalCoordinationDetection`: - `meanPhaseOffset phasesA phasesB : double option` Returns the argument (angle) of the mean complex phase- difference vector whose magnitude is the PLV. Returns None when series are empty, mismatched-length, or when the mean vector has effectively zero magnitude (1e-12 floor) — in which case direction is mathematically undefined. - `phaseLockingWithOffset phasesA phasesB : struct (double * double) option` Returns both magnitude and offset in one sequence pass. Zero-magnitude case: magnitude near 0, offset = nan; near-zero magnitude is the caller's reliable "offset is undefined" signal. Existing `phaseLockingValue` contract unchanged; new primitives are additive. Downstream `Graph.coordinationRiskScore*` and any other detector consuming PLV can now add a separate offset- based term instead of collapsing both into one scalar (Amara's explicit recommendation in correction #6). 8 new xUnit tests covering: - Identical series (offset = 0) - Constant pi/4 offset (observed = -pi/4, a-minus-b convention) - Anti-phase series (magnitude 1, offset = pi) — the correction #6 regression test, contrasted against in-phase (offset 0) with identical magnitude - Uniformly-distributed differences (zero-magnitude => None) - Empty / mismatched-length / single-element edge cases - phaseLockingWithOffset magnitude matches phaseLockingValue (consistency property preventing silent detector divergence) - phaseLockingWithOffset zero-magnitude returns (near-zero, nan) - phaseLockingWithOffset returns None on empty/mismatched All 37 TemporalCoordinationDetection tests pass locally. 0 Warnings / 0 Errors build. 6th of the 10 18th-ferry corrections operationalized this week (after test-classification doc in #339, parser-tech in #338). Remaining: Wilson CIs in CartelToy tests (needs #323 landed), MAD=0 percentile-rank fallback (needs #333 landed), conductance-sign doc (needs #331 landed), artifact-output layout (Stage-2 with calibration harness). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AceHack
added a commit
that referenced
this pull request
Apr 24, 2026
…340) * core: PLV mean phase offset — 19th graduation (Amara 18th-ferry #6) Addresses Amara 18th-ferry correction #6: PLV = 1 can mean anti-phase locking, not same-time synchronization. Downstream detectors that rely on "PLV = 1 => synchronized" misread anti-phase coordinators as same-time coordinators. Two new functions in `TemporalCoordinationDetection`: - `meanPhaseOffset phasesA phasesB : double option` Returns the argument (angle) of the mean complex phase- difference vector whose magnitude is the PLV. Returns None when series are empty, mismatched-length, or when the mean vector has effectively zero magnitude (1e-12 floor) — in which case direction is mathematically undefined. - `phaseLockingWithOffset phasesA phasesB : struct (double * double) option` Returns both magnitude and offset in one sequence pass. Zero-magnitude case: magnitude near 0, offset = nan; near-zero magnitude is the caller's reliable "offset is undefined" signal. Existing `phaseLockingValue` contract unchanged; new primitives are additive. Downstream `Graph.coordinationRiskScore*` and any other detector consuming PLV can now add a separate offset- based term instead of collapsing both into one scalar (Amara's explicit recommendation in correction #6). 8 new xUnit tests covering: - Identical series (offset = 0) - Constant pi/4 offset (observed = -pi/4, a-minus-b convention) - Anti-phase series (magnitude 1, offset = pi) — the correction #6 regression test, contrasted against in-phase (offset 0) with identical magnitude - Uniformly-distributed differences (zero-magnitude => None) - Empty / mismatched-length / single-element edge cases - phaseLockingWithOffset magnitude matches phaseLockingValue (consistency property preventing silent detector divergence) - phaseLockingWithOffset zero-magnitude returns (near-zero, nan) - phaseLockingWithOffset returns None on empty/mismatched All 37 TemporalCoordinationDetection tests pass locally. 0 Warnings / 0 Errors build. 6th of the 10 18th-ferry corrections operationalized this week (after test-classification doc in #339, parser-tech in #338). Remaining: Wilson CIs in CartelToy tests (needs #323 landed), MAD=0 percentile-rank fallback (needs #333 landed), conductance-sign doc (needs #331 landed), artifact-output layout (Stage-2 with calibration harness). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(#340): refactor shared accumulation + 5 review-thread fixes (Otto-216) Active PR-resolve-loop on #340 (PLV mean phase offset). 1. Sentinel-default in test (thread 59WGi9): replaced Option.defaultValue -1.0 pattern in the phaseLockingWithOffset-magnitude-matches-phaseLockingValue consistency test with explicit pattern-match + fail on None. Sentinel form would silently pass the equality assertion if BOTH primitives returned None, masking regressions. 2. Broken ferry cross-reference path (thread 59WGjn): doc comment referenced docs/aurora/2026-04-24-amara- calibration-ci-hardening-deep-research-plus-5-5- corrections-18th-ferry.md which doesn't exist on main (only 7th / 17th / 19th ferries landed as standalone docs). Rewrote provenance to describe the ferry topically + cross-reference the related 19th- ferry DST audit that IS in the repo. 3. Misleading "same PLV-magnitude floor" wording (thread 59WGj4): doc said meanPhaseOffset's zero-magnitude check uses "the same PLV-magnitude floor" — phaseLockingValue has NO floor (returns values arbitrarily close to 0). Fixed: clarified that the phasePairEpsilon floor applies ONLY to the offset-undefined decision; phaseLockingValue returns magnitude without threshold. 4. Name-attribution in doc comment (thread 59WGkP): "Aaron + Amara 11th ferry" replaced with "the 11th ferry" per factory role-reference convention. Audit- trail surfaces (commit messages, tick-history, memory) retain direct attribution; code/doc comments use role references. 5. Duplicate sin/cos accumulation across 3 functions (thread 59WGkn): extracted private helpers phasePairEpsilon + meanPhaseDiffVector. All three functions (phaseLockingValue, meanPhaseOffset, phaseLockingWithOffset) now route through the shared accumulator. Eliminates drift risk — one function can no longer silently diverge from the others on accumulation or threshold. Build: 0 Warning(s) / 0 Error(s). All 37 TemporalCoordinationDetection tests pass. All 5 threads replied via GraphQL next step. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(#340): 2 review threads (stale ferry path + atan2 range) Thread 59Yqkl (P1) — stale provenance reference: The doc cited `docs/aurora/2026-04-24-amara-temporal- coordination-detection-cartel-graph-influence-surface- 11th-ferry.md`, but the 11th ferry has not yet landed under `docs/aurora/` (it's queued in the Otto-105 operationalize cadence; PR #296 is its pending absorb). Replaced with the intent-preserving form: role references ("external AI collaborator's 11th courier ferry") plus a pointer at the MEMORY.md queue entry, so the provenance survives regardless of when the file-path question resolves. Also dropped the direct first-name so this factory-produced doc-comment tracks the name-attribution discipline. Thread 59YqlC (P2) — atan2 range correction: Doc said `(-pi, pi]` but `System.Math.Atan2` is documented as `[-pi, pi]` (both endpoints reachable under IEEE-754 signed-zero semantics: atan2(0, -1) = +pi, atan2(-0, -1) = -pi). Updated the doc to match the implementation. Behaviour unchanged. --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.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.
Summary
Binding ordering for external-language parser technology in Zeta per Aaron Otto-160 directive:
Captured cost distinction: ANTLR's build-time Java dependency (antlr4-tool.jar) is the main ergonomic friction; FParsec avoids it entirely.
Why this PR
Otto-160 directive. One row lands on its own branch to avoid positional-append conflict with PR #334 (pending merge, also touches the F# DSL row cluster).
Test plan
🤖 Generated with Claude Code