Render inline thread comments oldest-first (Fix #597) #604

Merged

crueber merged 1 commit from fix/issue-597 into main

2026-09-15 21:37:47 +00:00

crueber commented

2026-09-15 21:35:24 +00:00

Owner

ThreadComments rendered the wire array raw (newest-first by 02 §7 design), putting the newest reply farthest from the reply textbox. The now iterates chronological(getView()?.comments) — the #225 shared helper, no inline sort re-implementation. Wire byte-identical (no API/SDK change); ThreadComments is the only SPA consumer of pulls.threads.get().comments. Headless cover: web/test/unit/thread-comments-order-597.test.js (15 tests, incl. more:true window + missing-seq fallback + headless DOM newest-adjacent-above-reply). Full-minus-smoke green (1510 pass), vite + esbuild green, go vet clean. Doc amendment in docs/go/12_web_ui.md same commit (law 12). Fixes #597.

ThreadComments rendered the wire array raw (newest-first by 02 §7 design), putting the newest reply farthest from the reply textbox. The <For> now iterates chronological(getView()?.comments) — the #225 shared helper, no inline sort re-implementation. Wire byte-identical (no API/SDK change); ThreadComments is the only SPA consumer of pulls.threads.get().comments. Headless cover: web/test/unit/thread-comments-order-597.test.js (15 tests, incl. more:true window + missing-seq fallback + headless DOM newest-adjacent-above-reply). Full-minus-smoke green (1510 pass), vite + esbuild green, go vet clean. Doc amendment in docs/go/12_web_ui.md same commit (law 12). Fixes #597.

crueber added 1 commit

2026-09-15 21:35:25 +00:00

Render inline thread comments oldest-first via shared chronological() (Forgejo #597 ) e811d79b3e

ThreadComments rendered the wire array raw (newest-first by 02 §7
design), putting the newest reply farthest from the reply textbox.
The <For> now iterates chronological(getView()?.comments) — the #225
shared helper, no inline sort. Wire byte-identical; windowing caveat
noted in code. Headless cover: thread-comments-order-597.test.js.

crueber commented

2026-09-15 21:37:34 +00:00

Author

Owner

Independent review — APPROVED (no fix commits; no defects found).

Verified against #597 acceptance, in /tmp/walhub-597 on fix/issue-597 (e811d79), diff origin/main..origin/fix/issue-597 = Pull.jsx + thread-comments-order-597.test.js + 12_web_ui.md entry:

Oldest-first / newest-above-reply: ThreadComments now iterates chronological(getView()?.comments); ThreadCard source order (comments list above reply form) pinned. Headless DOM test asserts oldest→newest with no
between newest reply and reply input.
Reactivity/cache safety: chronological() copies via [...(events ?? [])] before sorting — confirmed non-mutating live (wire [3,2,1] intact after call), so the shared useData cache cannot be corrupted for other consumers. New array per evaluation only recomputes when getView() changes. No defect.
missing-seq fallback sane: seq-less rows sort first (-Inf), stable input order — matches the #225 convention, sensible since Seq is always present per model.go.
more:true window reasoning correct: window is newest-n; chronological within window is the right render; future older-pager caveat noted in code comment + doc.
Wire/SDK byte-identical: zero Go/SDK diff (only web/ + docs/ files in diff); GetThread newest-first + SDK GET-passthrough pins intact.
Single-consumer sweep confirmed independently: full web/src grep shows pulls.threads.get consumed only in Pull.jsx:950; PullFiles only calls threads.create (post), never renders .comments. (New test checks 3 files; extended sweep to all of web/src holds.)
Timelines untouched: ThreadTimeline.jsx + Issue.jsx zero diff; ThreadTimeline keeps the ONE #225 chronological sort.
New test fails pre-fix: chronological(getView()?.comments) absent on main, raw '?? []' present — both pins fail pre-fix; behavioral demo pre-fix renders 3,2,1 vs post-fix 1,2,3.
Laws: runtime deps exactly solid-js + @solidjs/router + marked + dompurify (law 1); #597 decision appended to docs/go/12_web_ui.md in same change (law 12); no CSS/order-utility delta (array order, not flex order).
Test runs: new file 15/15 green; thread-order suite 27/27; full node --test 1512/1512 non-smoke (1 smoke.test.js failure is environmental — needs a live server; amendment correctly scopes 'minus smoke'); vite build green. Restored web/dist/.keep after the local build so the worktree is clean.

Note: the DOM-order assertion uses a synthetic renderCard mirror (node --test has no Solid runtime) plus exact-string call-site pins — strongest available headless signal, consistent with repo convention. Acceptable, not a blocker.

Independent review — APPROVED (no fix commits; no defects found). Verified against #597 acceptance, in /tmp/walhub-597 on fix/issue-597 (e811d79), diff origin/main..origin/fix/issue-597 = Pull.jsx + thread-comments-order-597.test.js + 12_web_ui.md entry: - Oldest-first / newest-above-reply: ThreadComments <For> now iterates chronological(getView()?.comments); ThreadCard source order (comments list above reply form) pinned. Headless DOM test asserts oldest→newest with no <li> between newest reply and reply input. - Reactivity/cache safety: chronological() copies via [...(events ?? [])] before sorting — confirmed non-mutating live (wire [3,2,1] intact after call), so the shared useData cache cannot be corrupted for other consumers. New array per evaluation only recomputes when getView() changes. No defect. - missing-seq fallback sane: seq-less rows sort first (-Inf), stable input order — matches the #225 convention, sensible since Seq is always present per model.go. - more:true window reasoning correct: window is newest-n; chronological within window is the right render; future older-pager caveat noted in code comment + doc. - Wire/SDK byte-identical: zero Go/SDK diff (only web/ + docs/ files in diff); GetThread newest-first + SDK GET-passthrough pins intact. - Single-consumer sweep confirmed independently: full web/src grep shows pulls.threads.get consumed only in Pull.jsx:950; PullFiles only calls threads.create (post), never renders .comments. (New test checks 3 files; extended sweep to all of web/src holds.) - Timelines untouched: ThreadTimeline.jsx + Issue.jsx zero diff; ThreadTimeline keeps the ONE #225 chronological sort. - New test fails pre-fix: chronological(getView()?.comments) absent on main, raw '?? []' present — both pins fail pre-fix; behavioral demo pre-fix renders 3,2,1 vs post-fix 1,2,3. - Laws: runtime deps exactly solid-js + @solidjs/router + marked + dompurify (law 1); #597 decision appended to docs/go/12_web_ui.md in same change (law 12); no CSS/order-utility delta (array order, not flex order). - Test runs: new file 15/15 green; thread-order suite 27/27; full node --test 1512/1512 non-smoke (1 smoke.test.js failure is environmental — needs a live server; amendment correctly scopes 'minus smoke'); vite build green. Restored web/dist/.keep after the local build so the worktree is clean. Note: the DOM-order assertion uses a synthetic renderCard mirror (node --test has no Solid runtime) plus exact-string call-site pins — strongest available headless signal, consistent with repo convention. Acceptable, not a blocker.

crueber merged commit 43c6318210 into main

2026-09-15 21:37:47 +00:00

crueber referenced this pull request from a commit

2026-09-15 21:37:48 +00:00

Merge pull request 'Render inline thread comments oldest-first (Fix #597)' (#604) from fix/issue-597 into main

crueber referenced this pull request

2026-09-15 21:37:49 +00:00

Inline thread comments render newest-first, putting the newest reply farthest from the reply textbox #597