From cc90b4a3bd9fad3864033fc59ea2e440b4288c44 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Wed, 5 Aug 2026 04:23:25 +0300 Subject: [PATCH] fix(web): a code fence whose language changed says so (spec-by6.4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit prosediff hashes a block's Info, so ```go becoming ```python pairs the two fences as a modification — but Block.Lines holds a fence's contents without its delimiters, so the line script came out entirely equal and every row rendered as context. The page said the document changed and then showed nothing that had, which is worse than either saying nothing or showing the change: the reviewer looks for an edit that appears not to exist. The fence's opening delimiter is not a row of this table and inventing a line number for it would be a guess, so the change is stated as a marker row above the fence's lines, in the same shape a move already uses. Only a code fence is covered — Info also carries a list item's marker and a table's column count, and neither is a language a reviewer would want announced. --- .beads/interactions.jsonl | 2 ++ .beads/issues.jsonl | 4 ++-- web/diffrows.go | 39 +++++++++++++++++++++++++++++++++++- web/diffrows_test.go | 42 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 3 deletions(-) diff --git a/.beads/interactions.jsonl b/.beads/interactions.jsonl index 6926447b903f40890fa7cdae7af05c5001aeeefa..156282582e46e9907a2c56821dc9943e0a3c687a 100644 --- a/.beads/interactions.jsonl +++ b/.beads/interactions.jsonl @@ -14,3 +14,5 @@ {"id":"int-6f8f60b7e8df8f63b87952e47015d2d5","kind":"field_change","created_at":"2026-07-24T19:59:27.168688Z","actor":"Eugene Blikh","issue_id":"spec-by6.3.3","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Context blocks + anchor ids + thread UI; lost-anchor area; stale-block 409. Committed 472bcb1."}} {"id":"int-e8dbd67090e659150fb378d33b692d30","kind":"field_change","created_at":"2026-07-24T19:59:29.109943Z","actor":"Eugene Blikh","issue_id":"spec-by6.3","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"All four children done: core+db anchors (1a25367), service API + auto-merge gate (c6e5d66), mcpsrv spec_comment (f82d90a), web commentable diff (472bcb1)."}} {"id":"int-e7b60421ba107f7bcf5b5551535f1218","kind":"field_change","created_at":"2026-08-05T01:11:46.647218Z","actor":"Eugene Blikh","issue_id":"spec-by6.3.5","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Closed"}} +{"id":"int-91a8d0c6adf25db563af1304e26aab31","kind":"field_change","created_at":"2026-08-05T01:23:06.738792Z","actor":"Eugene Blikh","issue_id":"spec-by6.5","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Closed"}} +{"id":"int-89e435ba23e2faa4ea098a0d51bbdd0e","kind":"field_change","created_at":"2026-08-05T01:23:25.508715Z","actor":"Eugene Blikh","issue_id":"spec-by6.4","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Closed"}} diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 66793444f04ee286ef179ca264ab591bee0638aa..555b9e0daf711af9a22e6e20352e56c7a8471ea2 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -9,8 +9,8 @@ {"_type":"issue","id":"spec-43w","title":"Stale apk pins: paste.sr.ht 0.19.3-r0 and hub.sr.ht 0.29.4-r0 aged off the mirror","description":"Pre-existing, not caused by spec.sr.ht — but a FULL stack rebuild (post_push) would fail on these two apk layers and leave the stack un-deployable (the 2026-07-11 incident pattern). The self-hosted append-only repo.bigb.es mirror only preserves what it has seen, and it started after these aged off; old versions are unrecoverable.","design":"spec.sr.ht was deployed AROUND this with labng push --no-restart + a single-service 'docker compose build spec', so nothing else rebuilt. To fix properly: bump the two pins in versions.env to what the mirror currently ships (paste 0.19.4-r0, hub 0.31.1-r0 as of 2026-07-22). hub 0.29.4-\u003e0.31.1 crosses two minors with forward-only migrations — pg_dumpall first per the stack CLAUDE.md. Kept separate from the spec deploy deliberately.","status":"closed","priority":2,"issue_type":"bug","owner":"bigbes@gmail.com","created_at":"2026-07-23T04:09:54Z","created_by":"Eugene Blikh","updated_at":"2026-07-23T04:52:46Z","closed_at":"2026-07-23T04:52:46Z","close_reason":"Stale apk pins already bumped in the stack versions.env (paste 0.19.4-r0, hub 0.31.1-r0); fixed out of band.","dependencies":[{"issue_id":"spec-43w","depends_on_id":"spec-ejq","type":"parent-child","created_at":"2026-07-23T07:09:54Z","created_by":"Eugene Blikh","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"spec-ar4","title":"Restart other SourceHut services so spec.sr.ht appears in their nav","description":"The switcher entry is config-driven: each service builds its own nav from config sections ending in .sr.ht. spec.sr.ht is in the shared config.ini now, but the OTHER services (git, meta, todo, ...) only pick up the new entry on restart. Not done unprompted because it briefly bounces every service. On phoebe: docker compose restart \u003cthe sourcehut services\u003e.","notes":"2026-07-24: verified still needed, but BLOCKED on host access from this machine.\n\nVerified over HTTPS (LAN, read-only):\n- spec.srht.bigb.es -\u003e HTTP 200, the service itself is up.\n- git / meta / todo .srht.bigb.es nav all still MISSING a spec entry, so the restart has not happened and is still the remaining step.\n\nAccess attempts: 'phoebe' does not resolve; srht.bigb.es domains resolve to 192.168.88.18, which answers on :22 but refuses blikh (publickey,password). The working git remote is the restricted 'git' user shell, not a login. No deploy repo here carries an SSH path — deploys to phoebe are pull-based (apk-mirror re-indexes within 15 min), so there is no push-SSH route configured on this Mac.\n\nNeeds the owner to run it on phoebe directly:\n docker compose restart\nThen confirm with:\n curl -s https://git.srht.bigb.es | grep -c spec.srht.bigb.es # expect \u003e0","status":"in_progress","priority":2,"issue_type":"task","assignee":"Eugene Blikh","owner":"bigbes@gmail.com","created_at":"2026-07-23T04:09:53Z","created_by":"Eugene Blikh","updated_at":"2026-07-24T20:23:14Z","started_at":"2026-07-24T20:21:42Z","dependencies":[{"issue_id":"spec-ar4","depends_on_id":"spec-ejq","type":"parent-child","created_at":"2026-07-23T07:09:53Z","created_by":"Eugene Blikh","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"spec-3vz","title":"Phase 4: review plane — approve/reject in a browser","description":"Proposal pages at stable URLs (returned by every write), the inbox, prose diff, approve (merges immediately) / reject, status lifecycle, and the digest of policy-merged firehose content.","design":"Productionise prosediff (the Phase 0 spike) into the review UI. CRITICAL requirement from the Phase 0 verdict: 13% of real prose modifications shred into interleaved fragments (similarity \u003c= 0.73), so the UI MUST switch to a two-column old/new view below ~0.75 and inline word diffs above it — building only the inline renderer makes one review in eight unreadable. The prosediff package already emits the (Hash, HeadingPath, Ordinal) comment-anchor tuple. Reads a proposal branch via service.ReadDocumentAtRef (the deliberately-awkward bypass), never the normal read path.","status":"closed","priority":2,"issue_type":"feature","assignee":"Eugene Blikh","owner":"bigbes@gmail.com","created_at":"2026-07-23T04:09:25Z","created_by":"Eugene Blikh","updated_at":"2026-07-23T06:52:55Z","started_at":"2026-07-23T06:31:53Z","closed_at":"2026-07-23T06:52:55Z","close_reason":"Review plane shipped: proposal page at /~owner/space/p/\u003cid\u003e with the prose diff (inline word-diff above 0.75 similarity, two-column old/new below — the Phase 0 verdict's hard requirement, in web/diff.go), approve(merge)/reject as owner-only CSRF-guarded POSTs, status badges, and the /inbox review queue + policy-merged digest. service.ProposalDiff/InboxProposals/DigestProposals/MergeHuman back it; web.Reader extended. Diff reads the proposal branch by resolving its tip to a sha (pinned read) rather than the ReadDocumentAtRef bypass — cleaner and immutable. Validated: diff renderer unit tests (inline/two-column/escaping), proposal-page + approve/reject handler tests (CSRF, owner-only, stale-\u003e409, wrong-space-\u003e404), inbox tests, and PG-backed service tests. Follow-up: digest_mark 'since last seen' tracking.","dependencies":[{"issue_id":"spec-3vz","depends_on_id":"spec-ejq","type":"parent-child","created_at":"2026-07-23T07:09:25Z","created_by":"Eugene Blikh","metadata":"{}"},{"issue_id":"spec-3vz","depends_on_id":"spec-zqb","type":"blocks","created_at":"2026-07-23T07:09:26Z","created_by":"Eugene Blikh","metadata":"{}"}],"dependency_count":1,"dependent_count":1,"comment_count":0} -{"_type":"issue","id":"spec-by6.5","title":"prosediff: an equal span's Space flag is read from the old side only","description":"prosediff/token.go:207, spans(): Span.Space is taken from toks[0].Space, and for an equal run those tokens come from the OLD side. An insertion at the head of a block therefore emits\n\n op=insert space=false text=\"w16 w17\"\n op=equal space=false text=\"w12 w13\"\n\nand any renderer joining the script writes the two runs together with no space: {+w16 w17+}w12 w13.\n\nNot reachable from the web layer today — the new side's separator is simply not in the data the web layer is handed, and web/diffrows.go's sideSpans already carries a dropped span's separator onto the next kept one for the region rows. It affected the old block-card renderer identically, so it is not a regression from the line-numbered diff port (spec-by6.3.5).\n\nFix shape: OR the two sides' leading-token flags when building an equal span. Found by the property test added in web/diffrows_test.go, which is deliberately blind to spacing (it compares nonce word sequences) and says so.","status":"open","priority":3,"issue_type":"bug","owner":"bigbes@gmail.com","created_at":"2026-08-05T01:10:14Z","created_by":"Eugene Blikh","updated_at":"2026-08-05T01:10:14Z","dependencies":[{"issue_id":"spec-by6.5","depends_on_id":"spec-by6","type":"parent-child","created_at":"2026-08-05T04:10:14Z","created_by":"Eugene Blikh","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"spec-by6.4","title":"A code fence whose only change is its info string renders as unchanged","description":"prosediff.Block.Lines excludes the fence delimiters, so a change from ```go to ```python produces no line-script difference. Stats counts the block as modified (BlocksModified: 1) while the unified diff table renders every one of its lines as ph-r-eq context. The page therefore says 'this document changed' and then shows nothing that changed.\n\nFound by adversarial verification of spec-by6.3.5; pre-existing in the segmenter, not introduced by the line-numbered diff port. It matters more now than it did under the block-card UI, because the table's whole contract is that a row states what happened to that line.\n\nFix is in prosediff: either carry the info string into the block's diffed content, or have the web layer render an explicit marker row when Block.Info differs across a ChangeModify.","status":"open","priority":3,"issue_type":"bug","owner":"bigbes@gmail.com","created_at":"2026-08-05T00:49:06Z","created_by":"Eugene Blikh","updated_at":"2026-08-05T00:49:06Z","dependencies":[{"issue_id":"spec-by6.4","depends_on_id":"spec-by6","type":"parent-child","created_at":"2026-08-05T03:49:05Z","created_by":"Eugene Blikh","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"spec-by6.5","title":"prosediff: an equal span's Space flag is read from the old side only","description":"prosediff/token.go:207, spans(): Span.Space is taken from toks[0].Space, and for an equal run those tokens come from the OLD side. An insertion at the head of a block therefore emits\n\n op=insert space=false text=\"w16 w17\"\n op=equal space=false text=\"w12 w13\"\n\nand any renderer joining the script writes the two runs together with no space: {+w16 w17+}w12 w13.\n\nNot reachable from the web layer today — the new side's separator is simply not in the data the web layer is handed, and web/diffrows.go's sideSpans already carries a dropped span's separator onto the next kept one for the region rows. It affected the old block-card renderer identically, so it is not a regression from the line-numbered diff port (spec-by6.3.5).\n\nFix shape: OR the two sides' leading-token flags when building an equal span. Found by the property test added in web/diffrows_test.go, which is deliberately blind to spacing (it compares nonce word sequences) and says so.","status":"closed","priority":3,"issue_type":"bug","owner":"bigbes@gmail.com","created_at":"2026-08-05T01:10:14Z","created_by":"Eugene Blikh","updated_at":"2026-08-05T01:23:07Z","closed_at":"2026-08-05T01:23:07Z","close_reason":"Closed","dependencies":[{"issue_id":"spec-by6.5","depends_on_id":"spec-by6","type":"parent-child","created_at":"2026-08-05T04:10:14Z","created_by":"Eugene Blikh","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"spec-by6.4","title":"A code fence whose only change is its info string renders as unchanged","description":"prosediff.Block.Lines excludes the fence delimiters, so a change from ```go to ```python produces no line-script difference. Stats counts the block as modified (BlocksModified: 1) while the unified diff table renders every one of its lines as ph-r-eq context. The page therefore says 'this document changed' and then shows nothing that changed.\n\nFound by adversarial verification of spec-by6.3.5; pre-existing in the segmenter, not introduced by the line-numbered diff port. It matters more now than it did under the block-card UI, because the table's whole contract is that a row states what happened to that line.\n\nFix is in prosediff: either carry the info string into the block's diffed content, or have the web layer render an explicit marker row when Block.Info differs across a ChangeModify.","status":"closed","priority":3,"issue_type":"bug","owner":"bigbes@gmail.com","created_at":"2026-08-05T00:49:06Z","created_by":"Eugene Blikh","updated_at":"2026-08-05T01:23:25Z","closed_at":"2026-08-05T01:23:25Z","close_reason":"Closed","dependencies":[{"issue_id":"spec-by6.4","depends_on_id":"spec-by6","type":"parent-child","created_at":"2026-08-05T03:49:05Z","created_by":"Eugene Blikh","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"spec-by6.3.4","title":"mcpsrv: spec_comment tool (read + reply, no resolve)","description":"The agent half of the loop. spec_comment lets an agent list the comments on a proposal it authored and reply to them; it must NOT resolve one, since an agent marking its own critique resolved defeats the auto-merge gate. Reuses the Phase 3 provenance path (agent + agent_session) for reply authorship, and the existing owner+agents read gate from spec-jjo. mcpsrv/mcpsrv.go:39 still documents spec_comment as deliberately absent — update that comment when it lands.","notes":"Implemented. Divergence from this issue's original text, accepted: reads are NOT narrowed to 'a proposal the agent authored'. Agent identity is self-declared (X-Agent/X-Agent-Session headers) and all agents share one token, so an authorship check would gate on a string the caller chooses — stricter on paper than the owner+agents read plane (spec-jjo) it sits in, while enforcing nothing. Revisit if/when per-agent token scoping lands (spec-by6 backlog).\n\nNo-resolve is enforced by TYPE: Commenter names only Threads/ReplyTo/GetProposal/ProposalDiff, so service.CommentOn and service.ResolveThread are unreachable from the handler however service/ grows. Writer is now Proposer+Commenter.\n\nExtra beyond the issue: replying requires proposal+thread and checks membership, because a thread id is global and service.ReplyTo takes only the id — a mistyped id would otherwise reply on a stranger's proposal.","status":"closed","priority":3,"issue_type":"task","owner":"bigbes@gmail.com","created_at":"2026-07-24T18:57:18Z","created_by":"Eugene Blikh","updated_at":"2026-07-24T19:44:55Z","closed_at":"2026-07-24T19:44:55Z","close_reason":"spec_comment: list threads with anchor state + reply; no-resolve enforced by interface. 9 tests green, no DB needed. Committed f82d90a.","dependencies":[{"issue_id":"spec-by6.3.4","depends_on_id":"spec-by6.3.2","type":"blocks","created_at":"2026-07-24T21:57:30Z","created_by":"Eugene Blikh","metadata":"{}"},{"issue_id":"spec-by6.3.4","depends_on_id":"spec-by6.3","type":"parent-child","created_at":"2026-07-24T21:57:17Z","created_by":"Eugene Blikh","metadata":"{}"}],"dependency_count":1,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"spec-by6.2","title":"Attachments: image embeds render as visibly missing","description":"gitx walks .md only, so an archive built from a git tree has no attachment index and ![[diagram.png]] resolves to a visibly-missing link. doc.FromPages already accepts an assets map — the gap is producing one. Two options: (a) gitx.WalkBlobs + an asset index threaded through service.Archive, plus a read-plane route serving the blob; (b) adopt Mermaid-by-convention and document that binary attachments are out of scope. Carried over from spec-wcr item 7 (triaged 2026-07-24: a feature, not a latent bug).","status":"open","priority":3,"issue_type":"feature","owner":"bigbes@gmail.com","created_at":"2026-07-24T18:12:24Z","created_by":"Eugene Blikh","updated_at":"2026-07-24T18:12:24Z","dependencies":[{"issue_id":"spec-by6.2","depends_on_id":"spec-by6","type":"parent-child","created_at":"2026-07-24T21:12:24Z","created_by":"Eugene Blikh","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"spec-45j","title":"Phase 5a-2: webhook tables, SDL, resolvers, delivery on proposal open/merge/reject","description":"On top of 5a-1: gql_proposal_wh_sub/_delivery tables + migration, WebhookSubscription/Delivery/Payload SDL, create/delete mutations + webhooks queries, and fire via webhooks.Schedule on proposal lifecycle events (service event hook -\u003e queue). Reference pages.sr.ht webhooks/webhooks.go + graph/model/webhooks.go.","status":"closed","priority":3,"issue_type":"feature","owner":"bigbes@gmail.com","created_at":"2026-07-24T02:05:29Z","created_by":"Eugene Blikh","updated_at":"2026-07-24T09:50:16Z","closed_at":"2026-07-24T09:50:16Z","close_reason":"Webhook tables/SDL/resolvers/delivery all shipped and verified end-to-end.","dependencies":[{"issue_id":"spec-45j","depends_on_id":"spec-3m9","type":"parent-child","created_at":"2026-07-24T05:05:30Z","created_by":"Eugene Blikh","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} diff --git a/web/diffrows.go b/web/diffrows.go index efd9a3e2c96fc1b5e6928927fa8d3761784c5b87..1280fc0e956b6a442cecec23d4a2981fa07a6e62 100644 --- a/web/diffrows.go +++ b/web/diffrows.go @@ -303,7 +303,7 @@ func wholeBlockRows(src *prosediff.Block, kind rowKind, old bool, blk *rowBlock) // cannot be trusted — falls back to a pair of region rows. func modifyRows(c prosediff.BlockChange, blk *rowBlock) []diffRow { if len(c.Lines) > 0 { - return lineScriptRows(c, blk) + return append(infoRows(c, blk), lineScriptRows(c, blk)...) } if c.Similarity >= inlineSimilarityThreshold { if old, nw, ok := prosediff.WordsByLine(c); ok { @@ -313,6 +313,43 @@ func modifyRows(c prosediff.BlockChange, blk *rowBlock) []diffRow { return regionRows(c, blk) } +// infoRows is the one marker a modified code fence may need: the language on its +// opening delimiter changed. +// +// prosediff hashes a block's Info, so ```go becoming ```python pairs the two +// fences as a modification — but Block.Lines holds the fence's contents without +// its delimiters, so the line script is entirely equal and every row renders as +// context. The page would then say the document changed and show nothing that +// did. The fence delimiters are not rows of this table and inventing a number +// for one would be a guess, so the change is stated as a marker instead. +// +// Only a code fence is covered. Info also carries a list item's marker and a +// table's column count, and neither is a language: "- became *" is noise, and a +// table whose column count changed already differs in its cells. +func infoRows(c prosediff.BlockChange, blk *rowBlock) []diffRow { + if c.Old.Kind != prosediff.KindCode || c.Old.Info == c.New.Info { + return nil + } + return []diffRow{{ + Kind: rowMove, + Block: blk, + Note: fmt.Sprintf("code block: %s → %s", infoLabel(c.Old.Info), infoLabel(c.New.Info)), + }} +} + +// infoLabel spells the two Info values that are states rather than languages: a +// bare ``` fence, and an indented block, which the segmenter records as +// "indented" and which has no delimiter line at all. +func infoLabel(info string) string { + switch info { + case "": + return "no language" + case "indented": + return "indented, unfenced" + } + return info +} + // lineScriptRows renders a modified non-prose block. prosediff.DiffLines emits // exactly one span per line, so the two counters walk the two revisions in step // and every row's number is read off, not derived. diff --git a/web/diffrows_test.go b/web/diffrows_test.go index bfe05639c16f2fd7b3983692c6cf2effbb89ce18..f5fb7f5f570da39f35cad1443b5948b8449d26e7 100644 --- a/web/diffrows_test.go +++ b/web/diffrows_test.go @@ -319,6 +319,48 @@ func TestModifiedCodeFenceFollowsTheLineScript(t *testing.T) { } } +// A fence whose language changed and whose body did not used to render as a +// screen of context rows: prosediff hashes Info, so the two fences pair as a +// modification, but Block.Lines excludes the delimiter the language is written +// on, so the line script is entirely equal. The page said the document changed +// and then showed nothing that had. The marker states it instead — the +// delimiter is not a row of this table, and giving it a number would be a guess. +func TestCodeFenceLanguageChangeIsStated(t *testing.T) { + const old = "# H\n\n```go\na := 1\n```\n" + const nw = "# H\n\n```python\na := 1\n```\n" + + checkRows(t, modifyRowsOf(t, old, nw), []string{ + "move ·/· code block: go → python", + "eq 4/4 a := 1", + }) +} + +// A fence that loses its language keeps the marker readable: an empty Info is a +// bare ``` fence, which is a state and not a missing value. +func TestCodeFenceLosingItsLanguageSaysSo(t *testing.T) { + const old = "# H\n\n```go\na := 1\n```\n" + const nw = "# H\n\n```\na := 1\n```\n" + + checkRows(t, modifyRowsOf(t, old, nw), []string{ + "move ·/· code block: go → no language", + "eq 4/4 a := 1", + }) +} + +// A body edit inside a fence whose language did not change gets no marker: the +// line rows already say everything that happened. +func TestUnchangedFenceLanguageAddsNoMarker(t *testing.T) { + const old = "# H\n\n```go\na := 1\nb := 2\nc := 3\n```\n" + const nw = "# H\n\n```go\na := 1\nb := 20\nc := 3\n```\n" + + checkRows(t, modifyRowsOf(t, old, nw), []string{ + "eq 4/4 a := 1", + "del 5/· b := 2", + "ins ·/5 b := 20", + "eq 6/6 c := 3", + }) +} + // A move renders as a marker where the block was and the block itself where it // now is. The marker at the old position is not commentable: the text is on the // page once, and anchoring it twice would give one paragraph two places to be