fix: classify loom attributes structurally, report bare key= - #16
Open
JeremyGracey-AI wants to merge 1 commit into
Open
fix: classify loom attributes structurally, report bare key=#16JeremyGracey-AI wants to merge 1 commit into
key=#16JeremyGracey-AI wants to merge 1 commit into
Conversation
parseBrick decided attribute-vs-positional with `t.includes(':')`. Two bugs
fall out of that, both silent.
A positional string containing a colon was classified as an attribute and then
split at `indexOf(':')`, so `text "Runs 9:00 to 17:00"` produced
`{'"Runs 9': '00 to 17:00"'}` and `content` came back undefined. The text did
not render wrong, it disappeared. Times, URLs and ordinary prose all hit this.
parsePhase had the same test and lost phase descriptions the same way.
A `key="value"` token, the natural mistake for anyone coming from HTML, JSX or
weft itself, matched no colon at all and was absorbed into positional content.
A published page rendered the literal text `title="Verify evidence"` to
visitors, with no diagnostic at edit, save, publish or render.
Classify on `^[A-Za-z][A-Za-z0-9_.-]*:` instead, so a token is an attribute
only when it starts with one, and push a LoomParseError naming the key when a
bare `key=` appears. state.errors and LoomParseError already exist and are used
two lines up for `Unknown brick`, so this reaches for the channel that is
already there rather than adding one.
CONTRIBUTING says do not add silent fallbacks and fail loud; this is that rule
applied to the loom parser. The remedy also matches the roadmap's explicit
expand/gather item, which makes the same kind of implicit behaviour explicit
because AI writers confuse it and it produces wrong data quietly.
JeremyGracey-AI
added a commit
to JeremyGracey-AI/governance-drift-researcher
that referenced
this pull request
Aug 20, 2026
Submitted as WeaveMindAI/weft#16. The drafts stay as the record of what went out; the issue draft is kept because it describes the bug without the patch.
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
1 participant
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.
parseBrickdecided attribute-vs-positional witht.includes(':'). Two silent failures fall out of that.A positional string containing a colon was destroyed. It got classified as an attribute and split at
indexOf(':'), sotext "Runs 9:00 to 17:00"produced{'"Runs 9': '00 to 17:00"'}andcontentcame backundefined. The text didn't render wrong, it disappeared. Times, URLs and ordinary prose all hit it.parsePhasehad the same test and lost phase descriptions the same way.A
key="value"token was absorbed into body text. No colon, so it fell to the positional branch. A published page of mine rendered the literal stringtitle="Verify evidence"to visitors for a day, with no diagnostic at edit, save, publish or render. The loom had been generated by the Tangle Runner Page Builder, which is the part I'd flag hardest: the builder makes this mistake and nothing catches it.The change
Classify on
^[A-Za-z][A-Za-z0-9_.-]*:, so a token is an attribute only when it starts with one, and push aLoomParseErrornaming the key when a barekey=appears.state.errorsandLoomParseErroralready exist and are used two lines up forUnknown brick, so this reaches for the channel that's already there rather than adding one.I read this as applying CONTRIBUTING's "do not add silent fallbacks, fail loud" to the loom parser, and as the same remedy the roadmap already picked for explicit expand/gather: implicit behaviour that AI writers confuse and that produces wrong data quietly, made explicit.
Tests
New
dashboard/src/lib/ai/loom-parser.test.ts, 9 cases. 6 fail onmainand pass with the change; the other 3 are regression guards for behaviour that already worked (ordinary attributes, attribute values containing colons,=inside a quoted string).Checks
npx vitest run src/lib/ai/loom-parser.test.ts— 9 passedpnpm -C dashboard check— 4850 files, 0 errors, 0 warningsvitest run— 263 pre-existing failures onmainand on this branch, all catalog-dependent (I hadn't runscripts/catalog-link.sh). Delta is +9 passing, no change in failures.Reproduction without applying the patch
tokenizeLine,parseAttributesandparseBrick's classifier transcribed verbatim frommain, with cases: loom-parser-repro.mjs.node loom-parser-repro.mjs, no account or clone needed.Fuller write-up of how this surfaced, plus a few smaller publish-flow observations that are not part of this PR: UPSTREAM-REPORT.md.
Known remaining edge, not addressed here
Bracket-group attributes (
[label:"a:b"]) are expanded bytokenizeLineand still run throughparseAttributes'sindexOf(':'), so a colon inside a bracketed value can still split in the wrong place. Left alone to keep this to one function. Happy to follow up if you want it in scope.On the roadmap
I know "Centralize parsing and compilation in Rust" covers this file. This is deliberately small enough to carry over as test cases if the TypeScript parser goes away. Close it if you'd rather not touch the dashboard parser at all, no hard feelings, the bug report stands on its own.