edufeed/wp-plugin-amb-nostr
Watch
1
0
Fork
You've already forked wp-plugin-amb-nostr
 0
Code Issues 1 Pull requests Projects Releases Packages Wiki Activity Actions

feat(amb-nostr): ExtTagParser for the NIP-AMB ext: grammar, plus a test harness #4

Merged
laoc merged 2 commits from feature/ext-tag-parser into main 2026-07-31 20:18:51 +00:00
Conversation 1 Commits 2 Files changed 11 +2588 -3
laoc commented 2026-07-31 20:01:19 +00:00
Owner
Copy link

First prerequisite for the server-rendered archive/facet/detail routes. Pure PHP, no WordPress, no network — needed identically under either architecture option, so it is decision-independent and safe to land before that call is made.

What

ExtTagParser implements the normative left-anchored grammar from nips/AMB.md:

ext-key = "ext" ":" ns ":" facet [ ":" sub ]
sub     = "id" / "type" / "name" / "prefLabel" ":" lang

<ns> and <facet> contain no colon, so keys are parsed with fixed arity and a key outside the grammar is dropped rather than guessed at. Rejection is per key — a legacy ext:ekw:konfi:themen:id vanishes while its conforming siblings on the same event still parse. That mirrors the relay-side reader (nostrlib parseExtKey).

Behaviour is deliberately identical to the reference JS reader in edufeed-app (src/lib/helpers/educational/parseExtensionTags.js), mixed facets included.

How it was verified

Not just "tests pass" — parity against the reference reader on live data. Both parsers were run over the same 100-event sample from amb-relay.edufeed.org (kind 30142, #l=ekw; 32 carry ext tags, 82 facet instances, 2 namespaces) and their canonicalised output is byte-identical.

That sample also proves the drop is a drop and not a mis-bucketing: no facet named konfi appears anywhere, while the migrated org.edufeed.ekw.konfi:* facets parse normally. Both classes are present in the committed fixture.

amb-nostr 21/21, 54 assertions   (PHP 8.3.32, PHPUnit 10.5.64, at 50bb5cd, clean tree)

Also in here

amb-nostr was the only module with no phpunit.xml and no tests/ — this adds both, copying shared/'s setup. composer update adds dev dependencies only; no runtime package changed. The one runtime-looking line in composer.lock is the local path repository recording the current branch name, which it does on every branch.

What to look at

  • The grammar edge cases in ExtTagParser::parseKey() — particularly that an illegal sub returns null rather than falling back to a scalar reading.
  • Whether accepting the legacy unprefixed ekw: shape (LEGACY_NAMESPACES) is right. AMB.md says consumers MAY during transition and the JS reader does; I followed the reference rather than diverging, but it is a judgement call and easy to remove.
  • parseKey() is public on purpose — facet discovery and filter building work from tag keys alone.

Nothing is wired into the plugin yet, so this changes no rendered output.

First prerequisite for the server-rendered archive/facet/detail routes. Pure PHP, no WordPress, no network — needed identically under either architecture option, so it is decision-independent and safe to land before that call is made. ## What `ExtTagParser` implements the normative left-anchored grammar from `nips/AMB.md`: ``` ext-key = "ext" ":" ns ":" facet [ ":" sub ] sub = "id" / "type" / "name" / "prefLabel" ":" lang ``` `<ns>` and `<facet>` contain no colon, so keys are parsed with fixed arity and a key outside the grammar is dropped rather than guessed at. **Rejection is per key** — a legacy `ext:ekw:konfi:themen:id` vanishes while its conforming siblings on the same event still parse. That mirrors the relay-side reader (nostrlib `parseExtKey`). Behaviour is deliberately identical to the reference JS reader in edufeed-app (`src/lib/helpers/educational/parseExtensionTags.js`), mixed facets included. ## How it was verified Not just "tests pass" — **parity against the reference reader on live data**. Both parsers were run over the same 100-event sample from `amb-relay.edufeed.org` (kind 30142, `#l=ekw`; 32 carry ext tags, 82 facet instances, 2 namespaces) and their canonicalised output is byte-identical. That sample also proves the drop is a *drop* and not a mis-bucketing: no facet named `konfi` appears anywhere, while the migrated `org.edufeed.ekw.konfi:*` facets parse normally. Both classes are present in the committed fixture. ``` amb-nostr 21/21, 54 assertions (PHP 8.3.32, PHPUnit 10.5.64, at 50bb5cd, clean tree) ``` ## Also in here `amb-nostr` was the only module with no `phpunit.xml` and no `tests/` — this adds both, copying `shared/`'s setup. `composer update` adds dev dependencies only; no runtime package changed. The one runtime-looking line in `composer.lock` is the local path repository recording the current branch name, which it does on every branch. ## What to look at - The grammar edge cases in `ExtTagParser::parseKey()` — particularly that an illegal `sub` returns null rather than falling back to a scalar reading. - Whether accepting the legacy unprefixed `ekw:` shape (`LEGACY_NAMESPACES`) is right. `AMB.md` says consumers MAY during transition and the JS reader does; I followed the reference rather than diverging, but it is a judgement call and easy to remove. - `parseKey()` is public on purpose — facet discovery and filter building work from tag keys alone. Nothing is wired into the plugin yet, so this changes no rendered output. 
First of the prerequisites for the server-rendered archive and facet routes.
Pure PHP, no WordPress, no network — needed identically whether we serve pages
straight off the relay or off a local index, so it is decision-independent.

Implements the normative left-anchored grammar from nips/AMB.md:

    ext-key = "ext" ":" ns ":" facet [ ":" sub ]
    sub     = "id" / "type" / "name" / "prefLabel" ":" lang

`<ns>` and `<facet>` contain no colon, so keys are parsed with fixed arity and
a key outside the grammar is dropped rather than guessed at. Rejection is per
key: a legacy `ext:ekw:konfi:themen:id` on an event vanishes while its
conforming siblings on the same event still parse. That mirrors the relay-side
reader (nostrlib parseExtKey) and is asserted on real corpus data, not just on
hand-written tags.

Behaviour is deliberately identical to the reference JS reader in edufeed-app
(`src/lib/helpers/educational/parseExtensionTags.js`), mixed facets included —
concepts and scalars accumulate independently under one (ns, facet) pair. The
two readers drifting apart is what the NIP's parsing rule exists to prevent.

Verified as parity, not just as passing tests: both parsers were run over the
same 100-event live sample from amb-relay (kind 30142, `#l=ekw`, 32 events
carrying ext tags, 82 facet instances across two namespaces) and their
canonicalised output is byte-identical. That sample also confirms the drop is a
drop and not a mis-bucketing — no facet named `konfi` appears anywhere, while
the migrated `org.edufeed.ekw.konfi:*` facets parse normally.

`parseKey()` is public because facet discovery and filter building work from
tag keys alone, without reading values.

amb-nostr was the only module with no phpunit.xml and no tests/; this adds
both, copying shared/'s setup. `composer update` adds dev dependencies only —
no runtime package changed. The one runtime-looking line in composer.lock is
the local path repository recording the current branch name, which it does on
every branch.

    amb-nostr 21/21, 54 assertions (PHP 8.3.32, PHPUnit 10.5.64)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review follow-ups from TestOER on #4. None of these change ExtTagParser's
behaviour; the two documentation items and one pinning test are theirs
verbatim, the parity guard is the "costs an hour, makes the claim
permanent" item.

The parity check was a manual one-off, so either reader could drift and
nothing would catch it — the exact failure NIP-AMB's parsing rule exists
to prevent. It now runs the real reference reader from phpunit, and is
skipped rather than failed when node or EDUFEED_APP_PATH is absent.

Building it surfaced that the guard was initially vacuous: the live
fixture exercises no `:name` sub and produces no `mixed` facet, so
deleting either code path left the dumps identical. Added a 27-case
grammar corpus covering every branch, including the ones the live corpus
has no instance of. Verified by mutation, not by assumption:

  name-handling removed        -> parity FAILS
  mixed-kind derivation broken -> parity FAILS
  legacy namespace dropped     -> parity FAILS
  `type` sub rejected          -> parity passes, and correctly so: a
                                  `:type` value is ignored and a
                                  type-only facet is dropped either way,
                                  so no observable output differs

Also pins TestOER's two findings: integer-like ns/facet names become int
array keys in PHP and stay strings in JS (unfixable without abandoning
name-keyed arrays; zero live instances), and the shared reference
behaviour where a prefLabel after an empty `:id` attaches to the
preceding concept.

amb-nostr 26/26, 84 assertions (PHP 8.3.32 / PHPUnit 10.5.64), parity
group running against edufeed-app's parseExtensionTags.js.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
laoc commented 2026-07-31 20:18:40 +00:00
Author
Owner
Copy link

Review follow-ups from @TestOER applied in c8b430f. No behaviour change to ExtTagParser.

The parity guard — and the fact that it was vacuous when I first wrote it

Built the @group parity test as suggested: it runs the real reference reader from phpunit, skipping (not failing) when node or EDUFEED_APP_PATH is absent.

Then I mutated the parser to check the guard actually bites, and it didn't. Deleting the whole :name branch and breaking the mixed kind derivation both left the two dumps byte-identical, because the live fixture exercises neither — it contains no :name sub and produces no mixed facet. Exactly the point in your review: parity on live data is weaker than it sounds. My 100-event #l=ekw slice was denser than 8482 random events and still missed two whole code paths.

Fixed by adding a 27-case grammar corpus (tests/fixtures/parity-grammar.jsonl) covering every branch, including ones with no live instance. Re-verified by mutation:

mutation parity result
:name handling removed FAILS ✓
mixed kind derivation broken FAILS ✓
LEGACY_NAMESPACES emptied FAILS ✓
type sub rejected in parseKey passes — correctly

The last one is a genuine equivalence, not a hole: a :type value is ignored and a type-only facet is dropped either way, so no observable output differs. Nothing in the suite catches it, and nothing should.

Both readers agree on all 27 grammar cases — including mixed facets, :name, empty-:id, surplus segments and the legacy unprefixed shape. That is a much stronger claim than the original one.

Your two pinning items

  • Integer-like ns/facet names — pinned in testIntegerLikeNamesBecomeIntKeys and documented in the class docblock as a known divergence, with your reasoning (unfixable without abandoning name-keyed arrays; zero live instances; bites a downstream is_string()/=== on a facet name, not string interpolation).
  • LEGACY_NAMESPACES — kept, with a comment recording that it is unexercised by live data (0 unprefixed ekw: keys in 8482 events) and kept only to avoid diverging unilaterally from the reference reader.

Also pinned the shared empty-:id-then-prefLabel behaviour you flagged, as reference behaviour rather than a port artefact.

amb-nostr 26/26, 84 assertions (PHP 8.3.32 / PHPUnit 10.5.64), parity group live

On the konfi data, before it reaches @laoc_buzz

Your measurement reproduces exactly — I get 10 legacy-only, 1 migrated, 0 both on the #l=ekw slice. But "the migration script has effectively not been run" isn't quite the shape of it, and the distinction changes who can fix it:

The one migrated resource is authored by 776c7bfe… — the migration key. The 10 stuck ones are authored by 1c5ff3ca… (2) and 64278b3f… (8). A republish under the migration key lands at a different address (different pubkey), so it creates a parallel resource instead of replacing the legacy one — and I confirmed the migrated event's d tag collides with none of the 10.

So re-running the script cannot fix those 10. It needs the original signing keys, which is a custody question, not a scheduling one.

Review follow-ups from @TestOER applied in `c8b430f`. No behaviour change to `ExtTagParser`. ## The parity guard — and the fact that it was vacuous when I first wrote it Built the `@group parity` test as suggested: it runs the real reference reader from phpunit, skipping (not failing) when `node` or `EDUFEED_APP_PATH` is absent. Then I mutated the parser to check the guard actually bites, and **it didn't**. Deleting the whole `:name` branch and breaking the `mixed` kind derivation both left the two dumps byte-identical, because the live fixture exercises neither — it contains no `:name` sub and produces no `mixed` facet. Exactly the point in your review: parity on live data is weaker than it sounds. My 100-event `#l=ekw` slice was denser than 8482 random events and *still* missed two whole code paths. Fixed by adding a 27-case grammar corpus (`tests/fixtures/parity-grammar.jsonl`) covering every branch, including ones with no live instance. Re-verified by mutation: | mutation | parity result | |---|---| | `:name` handling removed | **FAILS** ✓ | | `mixed` kind derivation broken | **FAILS** ✓ | | `LEGACY_NAMESPACES` emptied | **FAILS** ✓ | | `type` sub rejected in `parseKey` | passes — **correctly** | The last one is a genuine equivalence, not a hole: a `:type` value is ignored and a type-only facet is dropped either way, so no observable output differs. Nothing in the suite catches it, and nothing should. Both readers agree on all 27 grammar cases — including mixed facets, `:name`, empty-`:id`, surplus segments and the legacy unprefixed shape. That is a much stronger claim than the original one. ## Your two pinning items - **Integer-like ns/facet names** — pinned in `testIntegerLikeNamesBecomeIntKeys` and documented in the class docblock as a known divergence, with your reasoning (unfixable without abandoning name-keyed arrays; zero live instances; bites a downstream `is_string()`/`===` on a facet *name*, not string interpolation). - **`LEGACY_NAMESPACES`** — kept, with a comment recording that it is unexercised by live data (0 unprefixed `ekw:` keys in 8482 events) and kept only to avoid diverging unilaterally from the reference reader. Also pinned the shared empty-`:id`-then-`prefLabel` behaviour you flagged, as reference behaviour rather than a port artefact. ``` amb-nostr 26/26, 84 assertions (PHP 8.3.32 / PHPUnit 10.5.64), parity group live ``` ## On the konfi data, before it reaches @laoc_buzz Your measurement reproduces exactly — I get **10 legacy-only, 1 migrated, 0 both** on the `#l=ekw` slice. But "the migration script has effectively not been run" isn't quite the shape of it, and the distinction changes who can fix it: The one migrated resource is authored by `776c7bfe…` — the migration key. The 10 stuck ones are authored by `1c5ff3ca…` (2) and `64278b3f…` (8). A republish under the migration key lands at a **different address** (different pubkey), so it creates a parallel resource instead of replacing the legacy one — and I confirmed the migrated event's `d` tag collides with none of the 10. So re-running the script cannot fix those 10. It needs the original signing keys, which is a custody question, not a scheduling one.
laoc merged commit a778496aac into main 2026-07-31 20:18:51 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
edufeed/wp-plugin-amb-nostr!4
No description provided.