Skip to content

fix(v0.12): acceptance findings: TOC toggle, raw GitHub URLs, PR header state - #243

Merged
jlevy merged 12 commits into
codex/v012-acceptancefrom
codex/v012-acceptance-fixes
Oct 3, 2026
Merged

jlevy merged 12 commits into
codex/v012-acceptancefrom
codex/v012-acceptance-fixes

Conversation

@jlevy

@jlevy jlevy commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

Fixes three findings from the v0.12 alpha acceptance (docs/project/qa/qa-2026-09-24-v012-alpha-acceptance.md): one commit per bead, then the review fixes listed under Review.

Base: #241

mb-ddbe (P2): the TOC drawer toggle scrolls away in a narrow pane

Cause. #preview-pane was both the scroller and, through transform: translateZ(0), the containing block for KPress's position: fixed floating UI. Fixed descendants of a transformed scroller are laid out in its scrolled content, so they move with it. The rule predates v0.12, so trusted folders had the same bug. KPress 0.3.5's components.css states the contract the pane broke: the viewport must not be a containing block, and a non-scrolling .kpress-frame must be.

Fix. The shell now wraps the pane in a non-scrolling div.preview-frame.kpress-frame. The frame carries the transform, the stacking level, and the pane shadow. The pane keeps the scroll, the kpress-doc container, and position: relative for absolutely positioned content. Print drops the frame's transform.

Tests.

  • The bead asked for a real-browser acceptance test. The repository has no browser harness (no Playwright), so that test is replaced by static checks plus a manual QA step, and the browser measurements below are the evidence for this change.
  • tests/test_preview_frame_contract.py pins, by stylesheet text, the wrapper, the frame's transform, and that no rule in the shell's, a built-in plugin's, or KPress's stylesheet gives the pane or an element between it and the TOC a property that makes a containing block for fixed descendants. It does not see computed styles or rules a script adds. Against the old stylesheet it reports .preview-pane { transform: translateZ(0) }.
  • The layout itself is a new paint-exempt parity row, document.floating-ui-frame.
  • QA runbook 5.9 step 5 now checks the toggle at every depth, in a mirror and in a trusted folder.

Browser measurements. The browser was the Claude desktop browser pane. Servers ran on ports 8701 (trusted docs/), 8702 (a file:// mirror of this branch, using the untrusted inert TOC), and 8703. The document was qa-v012-repository-library.md.

Case Before After
1000 px window, light, trusted, scrollTop 8292 toggle y = -8276 (off screen) toggle at (312, 16) inside the pane
Same, at 0, 2000, 8292, and the end (23848) — toggle at (312, 16) every time; elementFromPoint at its centre hits it
Drawer opened from the end of the document — drawer at (316, 64), 668×362, inside the pane; backdrop exactly the pane (300,0 700×800), not the nav; backdrop click and entry click close it
1000 px, dark, trusted — same geometry; toggle fill is --kpress-doc-bg at 80%
Mirror (inert TOC), 1000 px light, end at 23299 old layout, emulated: toggle y = -23282 toggle at (312, 16); drawer inside the pane; backdrop and entry close it
Mirror, 760 px dark (pane 460 px) — toggle at (312, 16) at the end (30818); drawer 428 px wide inside the pane
1700 px, light and dark, trusted and mirror rail at 96, then sticky at 32 identical rail, prose rects, and scrollHeight with the old CSS emulated at the same scroll; toggle display: none

Screenshots at 1000×700 show the toggle in the pane's corner after an 8292 px scroll, and the drawer open over the pane with the file tree undimmed.

Limit. The pane was hidden, so rendering was throttled. KPress's scroll-driven show state, the fades, and a TOC entry's own scroll step (Metabrowser waits two animation frames) did not run on their own. The checks dispatched scroll, finished the transitions, or set the class the inert TOC's frame callback would set. The old layout, emulated, behaved the same way, so this is not a regression. The toggle's geometry is what the fix changes, and it was measured directly.

mb-tals (P3): a raw Unicode character or space in a GitHub URL is refused without a hint

Decision: accept. A browser sends a space, or any character outside ASCII, in a path, query, or fragment percent-encoded as UTF-8. These are the WHATWG path, query, and fragment percent-encode sets. GitHub answers both spellings alike, so the reducer now does the same for https/http URLs. The raw and encoded spellings reduce to the same RepositorySelection.

Accepting also avoids the alternative's flaw: a refusal that printed the encoded URL would repeat the argument, and the reducer promises never to repeat it.

These stay refused, and keep their codes:

  • C0 and C1 controls;
  • whitespace other than a space, such as U+00A0 or U+3000;
  • format characters, such as U+202E and U+200D, Unicode's default-ignorable characters, such as U+3164 and U+FE0F, the blank braille pattern U+2800, and unassigned and private-use code points;
  • bytes that are not UTF-8;
  • a trailing space, which a browser strips;
  • a % that starts no percent escape, with the hint to write a literal % as %25.

Each character refusal now names the code point and, where one exists, the spelling to use, for example the URL contains U+202E, an invisible character; if it belongs in the address, write it as %E2%80%AE. SSH addresses keep the generic checks. github.com/octo/démo is now invalid_repository, since GitHub names are ASCII.

Goldens.

  • cli-github-url-open.txt opens docs/My Notes.md on the web and raw hosts, and docs/雪.md#L1 on a branch added to the stand-in origin. The test asserts each is byte-for-byte equal to its encoded spelling.
  • cli-github-urls.tryscript.md shows the raw forms accepted without Git, and the refusals by code point.
  • tests/test_github_url_reducer.py adds raw/encoded pairs and the exact refusal texts.

mb-5wqg (P3): the PR header says "wants to merge" when merged; skipped checks count as neutral

github.com's wording, read on 2026-09-24:

  • cli/cli#14128, merged: "babakks merged 6 commits into trunk from …". This names the merger; gh api gives user BagToad, merged_by babakks, and commits 6.
  • cli/cli#14502, closed and unmerged: "00200200 wants to merge 1 commit into cli:trunk".

describePull now gives the header's actor and action: the author and "wants to merge N commits into" while open, draft, or closed; the merger and "merged N commits into" once merged. A check run whose conclusion is skipped or cancelled gets its own tone and count, with a muted badge (see Review for the full table). When a merged record names no merger, the header says only "merged N commits into …". cli/cli#14128 would now read 13 success · 13 skipped.

The count and the merger come from the pulls/<n> answer the record already reads. The record gains commits and merged_by, and PULL_RECORD_SCHEMA becomes 2, so an earlier record is refetched rather than migrated, as the plan designs.

Tests.

  • The session recording adds what the same server answered for the stand-in's pull requests 8 (merged by octo, docs run skipped) and 9 (closed).
  • The browserless session prints states: the header and check counts for open, merged, and closed.
  • Goldens updated: cli-ui-github-pull-page.tryscript.md, cli-github-pull.tryscript.md, and cli-github-pull-refresh.txt. QA runbook 5.7, steps 1 and 8.
  • In the browser (port 8703, stand-in PR 8): the header reads octo merged 1 commit into topic from guide-more, and Checks reads 2 success · 1 skipped with a muted skipped badge (--muted).

Evidence

  • make lint-check passes, including parity: 38 routes covered, 5 exempt, 11 kinds, 48 functional aspects, 3 of them paint-exempt.
  • These focused pytest modules pass:
    • test_preview_frame_contract, test_preview_pane_state_js, test_kpress_print_contract, test_quick_file_integration, and the browser, shell, and CSS suites (330 tests);
    • test_github_url_reducer, test_cli_github_url_golden, test_github_provider, test_cli_main, and test_repository_cache_contract_fixtures;
    • test_cli_github_pull_golden, test_github_pulls, test_pull_page_route, and test_github_pull_page_session (84 tests).
  • These tryscript goldens pass: cli-github-urls (31), and cli-github-pull with cli-ui-github-pull-page (25).
  • The full make test was not run locally because the machine is heavily loaded. CI runs it.

Correction to mb-5wqg

The bead says a closed pull request "should not say 'wants to' either". That is not what github.com does. On cli/cli#14502, which is closed and unmerged, it says "00200200 wants to merge 1 commit into cli:trunk". This branch keeps "wants to merge" for closed pull requests and lets the Closed badge carry the state. The coordinator confirmed this.

Review

The independent review found no P0, P1, or P2. It found eight P3s, each fixed in its own commit:

  • P3-1, check counts (1dc07c38). Cancelled, stale, and conclusion-less runs, and runs whose status was waiting, requested, or empty, no longer count as neutral. checkRunResult and commitStatusResult now sort every check into one bucket:

    • pending: any run that has not completed, and a pending status;
    • success;
    • failure: failure, timed_out, action_required, startup_failure, and a status of error;
    • cancelled, skipped, stale, and neutral, each counted separately;
    • unknown: a completed run with no conclusion, or any undocumented value.

    github.com's Checks page labels a cancelled run on its own ("This job was cancelled"; checked on pytorch/pytorch#198558, whose head commit has 11 cancelled runs according to a read-only gh api GET). tests/test_github_check_tones.py checks every documented status, conclusion, and state against this table. The CHANGELOG lists exactly what is counted.

  • P3-2, invisible characters in URLs (e11f0eda). The reducer now also refuses:

    • Unicode's Default_Ignorable_Code_Point set, hardcoded from Unicode 17.0's DerivedCoreProperties.txt and checked against ICU 78.3;
    • U+2800 BRAILLE PATTERN BLANK;
    • unassigned (Cn) and private-use (Co) code points.

    So README<U+3164>.md no longer passes for README.md. Each refusal gives the encoded spelling, and a test checks that spelling opens the file. Which code points count as unassigned depends on the running Python's Unicode data (16.0 on Python 3.14).

  • P3-3, a lone % (e11f0eda). A % not followed by two hex digits is still refused. The WHATWG URL parser leaves such a % for the server to interpret, so it is ambiguous. The message now says to write a literal % as %25.

  • P3-4, format characters in output (85975a7f). display_segment now also replaces every format character (category Cf) with U+FFFD. That covers pinned-tree names, path: lines, and errors. The GitHub URL golden runs through metab: it opens docs/a<U+202E>b.md from its encoded URL, refuses docs/<U+202E>2J.md, and asserts no raw U+202E reaches the transcript. It fails on the old function.

  • P3-5, invisible characters in test source (845fbc0a). The reducer tests now write them as \u escapes. Ruff's PLE2502 and PLE2515 are enabled; src, tests, devtools, and explorations already pass them.

  • P3-6, the frame contract test (ae56bbfd). The scan now:

    • also checks translate, rotate, scale, transform-style: preserve-3d, content-visibility, layout or paint contain, and will-change;
    • covers every element between the pane and the TOC, as read from the rendered page;
    • reads the shell's CSS, every built-in plugin's CSS, and KPress's CSS.

    The parity row now says only what the test pins, and states that computed styles are not checked.

  • P3-7, plan selector (ae56bbfd). The menu-primitives plan now names .preview-frame as the transformed ancestor.

  • P3-8, three smaller fixes:

    • d288f08e caps commits at 1,000,000. The API mapping leaves a larger or negative count out, and the model rejects one.
    • 7d91c8f4: a merged record with no merged_by says "merged N commits into …" with no actor. The recording adds that answer, and the session prints it.
    • e11f0eda: every character refusal names the code point, including C0 controls (for example U+0009), a space, and a non-ASCII character in an SSH address.

Review evidence:

  • make lint-check passes.
  • 16 focused pytest modules pass, 490 tests in all: reducer, URL golden, git tree source, frame contract, check tones, pull data and page session, pull golden, pull route, preview pane, KPress print, docs discipline, provider, CLI, git content routes, and cache resolve.
  • The tryscript goldens cli-github-urls, cli-github-pull, and cli-ui-github-pull-page pass, 59 tests in all.

Acceptance rerun

Rerun on the installed wheel 0.11.1.dev412+7d91c8f4 (head 7d91c8f4, base 948861c4, main 6c278f3f), 2026-09-24 23:11–23:39 UTC. The record is the "Rerun on #243" section of docs/project/qa/qa-2026-09-24-v012-alpha-acceptance.md (df5bbbed).

Row Result
M02b: raw Unicode and space in tree, blob, raw, and #L URLs; U+202E, U+3164, and lone % refusals; the encoded spellings Pass
M07 wording: open (#234), merged (cli/cli#14128), closed (cli/cli#14502, encode/httpx#3783); skipped and cancelled checks Pass
TOC drawer (mb-ddbe): narrow in a trusted folder (light, dark) and a mirror (dark); wide layout in both themes unchanged against the old CSS; image, HTML, Source #L, and Git inside the frame Pass
M01 and M05 smoke (warm, then offline restart) Pass
M03b, M08b Not run: awaiting decision (mb-zb5t)

New P4 findings: mb-v8sb (the header omits the base owner when the head is in a fork: "into trunk", where github.com says "into cli:trunk") and mb-1bpe (a path_not_found error prints U+3164 raw).

🤖 Generated with Claude Code

Test and others added 10 commits September 24, 2026 14:48
In a pane too narrow for the Contents rail, the TOC drawer toggle scrolled
away with the document, so the drawer opened only from the top. The
scrolling #preview-pane also carried `transform: translateZ(0)`, which made
it the containing block for the toggle's `position: fixed`; fixed
descendants of a transformed scroller are laid out in its scrolled content.
Measured at a 1000 px window: the toggle sat at y = -8276 after an 8292 px
scroll.

The shell now wraps the pane in a non-scrolling `.preview-frame.kpress-frame`,
which carries the transform, the stacking level, and the pane shadow, as
KPress 0.3.5's embedding contract asks. The pane keeps the scroll, the
`kpress-doc` container, and its positioning for absolute content. Print
drops the frame's layer.

tests/test_preview_frame_contract.py pins the wrapper and that only the
frame is a containing block for fixed descendants; it fails on the old
shell. The layout itself is a paint-exempt row naming that test and QA
runbook 5.9 step 5, which now checks the toggle at every depth in a
trusted folder and a mirror.

Refs: mb-ddbe

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er would

A GitHub URL pasted as an address bar shows it, such as .../docs/雪.md or
.../space name.md, was refused as non_ascii or control_or_whitespace with no
way forward, while its percent-encoded spelling opened. A browser sends a
space and any character outside ASCII in a path, query, or fragment
percent-encoded as UTF-8 (the WHATWG path, query, and fragment
percent-encode sets), and GitHub answers both spellings alike, so the
reducer now does the same: the raw and the encoded spelling reduce to the
same selection.

What stays refused, keeping its code, is what an address bar would not
show raw: C0 and C1 controls, whitespace other than a space, invisible
formatting characters such as U+202E, bytes that are not UTF-8, and a
trailing space, which a browser strips. Each refusal names the code point
and, where one exists, the encoded spelling to use, without repeating the
argument. SSH addresses are not browser URLs and keep the generic checks.
An owner or repository with a raw non-ASCII letter is now refused by name
(invalid_repository), since GitHub's names are ASCII.

Goldens: cli-github-url-open.txt opens docs/My Notes.md (web and raw host)
and docs/雪.md#L1 on a branch of the stand-in origin, each byte for byte
equal to its encoded spelling; cli-github-urls.tryscript.md shows the raw
forms accepted without Git and the refusals by code point. The plan
records the decision and runbook 4.7 names the new blocks.

Refs: mb-tals

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The header said "<author> wants to merge into <base> from <head>" in every
state, and the Checks summary counted skipped runs as neutral: on
cli/cli#14128, merged with 13 success and 13 skipped runs, the page read
"wants to merge" beside the Merged badge and "13 success · 13 neutral".

github.com (read 2026-09-24) says "babakks merged 6 commits into trunk from
..." on that merged pull request, naming who merged it rather than its
author (BagToad), and "00200200 wants to merge 1 commit into cli:trunk" on
the closed, unmerged cli/cli#14502. So describePull now gives the header's
actor and action: the author and "wants to merge N commits into" while open,
draft, or closed, and the merger and "merged N commits into" once merged.
A check run whose conclusion is skipped gets its own tone and count, and the
same muted badge as neutral.

The count and the merger come from the pulls/<n> answer the record already
reads, as its `commits` and `merged_by`, so the record gains both and
PULL_RECORD_SCHEMA is 2: a record from an earlier build is refetched, as the
plan designs, rather than migrated.

The stand-in's pull request 8 is now merged by another account with its docs
run skipped. The session recording adds what the same server answered for
pull requests 8 and 9, and the browserless session prints the header and
the check counts for the open, merged, and closed states. Goldens and QA
runbook 5.7 steps 1 and 8 follow.

Refs: mb-5wqg

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tests/test_github_url_reducer.py held a literal U+00A0, U+3000, U+202E,
U+200D, U+E000, and U+0301 in its source, so its cases read as something
they are not. They are now \u escapes.

Ruff's PLE2502 (bidirectional control characters) and PLE2515 (zero-width
space) are enabled: src, tests, devtools, and explorations already pass
them, and the file's old text fails both.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… TOC

The frame contract test read only exact `.preview-pane` rules and six
properties, while the parity row claimed only the frame is a containing
block. It now reads the shell's stylesheet, every built-in plugin's, and
KPress's, and refuses any rule whose subject is the pane or an element
between it and the TOC (the content body and KPress host, the article, and
the document layout, as a trusted and a mirrored document render them),
when it sets transform, translate, rotate, scale, perspective,
transform-style: preserve-3d, filter, backdrop-filter, a layout or paint
contain, content-visibility: auto or hidden, or will-change naming any of
them. A table of cases shows it sees each property and ignores
pseudo-elements and descendants below the TOC; against the old
stylesheet it reports `.preview-pane { transform: translateZ(0) }`.

The parity row now says exactly that, and that computed styles and
script-added rules are not checked. The menu-primitives plan names
`.preview-frame` as the transformed ancestor; its reason stands.

Refs: mb-ddbe

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…one %

Review of #243. The reducer let through raw characters that render as
nothing, so README<U+3164>.md passed for README.md: it refused only
controls, format characters, and surrogates. It now also refuses Unicode's
Default_Ignorable_Code_Point set (hardcoded from Unicode 17.0's
DerivedCoreProperties.txt as ICU 78.3 reports it: the Hangul fillers, the
combining grapheme joiner, variation selectors, the tag block, and the
rest), U+2800 BRAILLE PATTERN BLANK, and unassigned (Cn) and private-use
(Co) code points. Each refusal names the code point and the encoded
spelling to use, which the tests check opens that name. Which code points
are unassigned follows the running Python's Unicode database.

A % that starts no percent escape, as in /blob/main/100%.md, is still
refused, since a browser leaves it for the server to interpret, but the
message now says to write a literal % as %25.

Every character refusal now names its code point, including a C0 control
("the URL contains U+0009, a control character") and a space or non-ASCII
character before the path or in an SSH address.

Refs: mb-tals

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review of #243. display_segment replaced C0, DEL, and C1 with U+FFFD, so a
path decoded from %E2%80%AE printed a raw right-to-left override on the
`path:` line and in a path_not_found error, reversing the text after it in
the terminal. It now also replaces every format character (category Cf):
bidirectional controls and isolates, zero-width spaces and joiners, the
byte-order mark, the soft hyphen, tags, and the rest. It is the display
form used for a pinned tree's names, the CLI's selection lines, and
errors; wire paths keep every byte. Variation selectors and combining
marks are not Cf and stay.

The GitHub URL golden now opens docs/a<U+202E>b.md from its encoded URL
and refuses docs/<U+202E>2J.md through metab, and asserts no U+202E or
U+009B reaches the transcript; both fail on the old function.

Refs: mb-tals

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review of #243. The record's `commits` had a floor and no ceiling. It is
now at most MAX_PULL_COMMITS, 1,000,000, far past any real pull request:
GitHub lists at most 250 of a pull request's commits but counts them all,
and documents no cap on that count. The API mapping leaves a count outside
0..MAX_PULL_COMMITS out (None) rather than refusing the record, and the
model refuses one on read. Tests cover both.

Refs: mb-5wqg

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… does

Review of #243. A cancelled or stale run, a completed run with no
conclusion, and a run whose status was waiting, requested, or empty were
all counted as neutral. github.com's Checks page names a cancelled run
apart ("This job was cancelled", beside "was skipped", "failed", and "This
check was neutral", read on pytorch/pytorch#198558, whose head has 11
cancelled runs by a read-only gh api GET).

checkRunResult and commitStatusResult now decide each check's tone and
label from two tables: a run not yet completed, whatever its status, is
pending; a completed run is success, failure (failure, timed_out,
action_required, startup_failure), cancelled, skipped, stale, or neutral
by its conclusion, and unknown with no conclusion or an undocumented one;
a commit status is success, failure (failure, error), or pending, and
unknown otherwise. Cancelled, stale, and unknown badges are muted like
neutral.

tests/test_github_check_tones.py runs both functions over every documented
status, conclusion, and state and the undocumented cases, and checks that
every tone has a badge color. The CHANGELOG states exactly what is counted.

Refs: mb-5wqg

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review of #243. A merged record whose merged_by is null rendered
"ghost merged 1 commit into ...", naming GitHub's placeholder for a
deleted account when the API named no account at all. The header now
says only "merged 1 commit into <base> from <head>" then, with no actor;
a merger GitHub reports keeps its name, and "ghost" stays for an author
the record does not name, as before.

The session recording adds what the same server answered after GitHub
answered pull request 8 with no merger, and the session prints its
header beside the open, merged, and closed ones.

Refs: mb-5wqg

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jlevy
jlevy marked this pull request as ready for review September 24, 2026 23:10
Reruns M02b, M07 wording, and the TOC drawer (mb-ddbe) on the installed
wheel from 7d91c8f, with an M01 and M05 smoke pass. All pass. M03b and
M08b are not run, awaiting the decision on mb-zb5t. Two P4 findings are
filed: mb-v8sb (the header omits the base owner for a fork) and mb-1bpe
(path errors print U+3164 raw).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…fter the linear restack

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jlevy pushed a commit that referenced this pull request Sep 25, 2026
…er the linear restack

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jlevy
jlevy added this pull request to stack #245 September 28, 2026 04:00
@jlevy
jlevy removed this pull request from stack #245 October 2, 2026 04:27
@jlevy
jlevy added this pull request to stack #218 October 2, 2026 04:27
@jlevy
jlevy merged commit aa97085 into main Oct 3, 2026
9 checks passed
@jlevy
jlevy deleted the codex/v012-acceptance-fixes branch October 3, 2026 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant