Skip to content

test: delete low-value and redundant suite files - #2462

Merged
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests
Aug 8, 2026
Merged

test: delete low-value and redundant suite files#2462
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests

Conversation

@jackwener

@jackwenerjackwener commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

Delete 102 low-value or redundant source test files across Desktop, UI, Runtime, Headless, CLI, and Core, reducing the source test-file surface from 988 to 886 (-10.3%).

This branch is now based on the current main. The cleanup deliberately keeps tests that are the only guard for an active dependency patch or a security-sensitive boundary.

Removed

  • Desktop markup, copy, presentation, chrome, and overlapping geometry/workbar coverage
  • UI localization/copy panels, quiet chrome, empty states, and presentation pins
  • Runtime thin wire and duplicate provider helper coverage
  • Headless duplicate A/B, toolchain, and prompt-summary coverage
  • CLI layout/completion coverage and Core copy/locale helpers

Kept after review

  • Active dependency-patch guards:
    • AI SDK tool-call index tracking
    • Astryx List accessible-name forwarding
    • Astryx new-message indicator behavior
  • Security-sensitive boundaries:
    • split-chunk tool-output secret redaction
    • permission escalation confirmation and persistence
    • task-ledger prompt injection/redaction wiring
    • Responses API store: false
    • session-title internal-instruction stripping
  • Root authority, credentials, storage safety, primary runtime/session/tool suites, and fail-safe CI planning

Additional change

  • CLI process-contract tests ignore Node's ExperimentalWarning: SQLite stderr noise while continuing to reject other stderr output.

Validation

  • git diff --check origin/main...HEAD
  • npm run format:check
  • npm test — all workspace tests passed after synchronizing with current main

GitHub CI is running on the updated head.

jackwenerand others added 8 commits August 8, 2026 01:38
Remove 122 source test files (~12% of packages/apps test files) that were
markup/copy contracts, overlapping e2e chrome, duplicate UI presentation
pins, and low-signal headless AB/toolchain matrix cases. Keep safety,
authority, and primary product unit paths.
Also tolerate Node SQLite ExperimentalWarning in CLI run process contracts
so stderr-empty assertions are not flaky on modern Node.
Bring back e2e journeys, headless matrix/export, catalog contract, AiSdkFlow,
and subscription non-leak coverage that are sole or primary gates. Keep the
remaining ~11.6% file-level reduction of true low-value markup/chrome/matrix
duplicates.
Keep the sole PROVIDER_REGISTRY×models.dev sync sweep and the provider
catalog tab/OAuth/slug contract. These are not markup-copy pins and have no
thinner remaining suite.
…lue-tests
# Conflicts:
#	apps/desktop/e2e/new-messages-indicator.spec.ts
#	apps/desktop/e2e/prompt-rail.spec.ts
#	packages/ui/src/__tests__/conversation-localization.test.tsx
@Astro-Han
Astro-Han merged commit 2f5c75b into mainAug 8, 2026
21 of 23 checks passed
@Astro-Han
Astro-Han deleted the cleanup/delete-low-value-tests branch August 8, 2026 03:28
UncertaintyDeterminesYou4ndMe pushed a commit to UncertaintyDeterminesYou4ndMe/maka-agent that referenced this pull request Aug 8, 2026
Part of apache#2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
apache#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Astro-Han pushed a commit that referenced this pull request Aug 8, 2026
Part of #2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Co-authored-by: 陆逊 <211125649+UncertaintyDeterminesYou4ndMe@users.noreply.github.com>
ARE404 added a commit to ARE404/maka-agent that referenced this pull request Aug 13, 2026
apache#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— apache#2161 pinned it against a containing block as tall as the conversation,
apache#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in apache#2462, and the multi-prompt fixtures it ran on in apache#2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
Astro-Han pushed a commit that referenced this pull request Aug 13, 2026
…2923)
* fix(ui): give the prompt rail's tick bars a box again
#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— #2161 pinned it against a containing block as tall as the conversation,
#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in #2462, and the multi-prompt fixtures it ran on in #2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
* fix(ui): make the prompt rail's hover and jump behave
Four things the rail got wrong once it was visible again, found by using it:
- A 4px gap between ticks was a band where the pointer was over the rail
and over no tick, so the dock-style hover falloff dropped out and picked
up again every few pixels of travel. The rail's `gap` moves into the
ticks' own `padding-block`: same pitch, hit boxes now tile.
- The hover preview waited 300ms before opening — Astryx's HoverCard
default, meant for a pointer crossing a wide row on its way somewhere
else. A tick is 22px of rail that nothing is on the way to, and the wait
is the one part of this hover with no motion in it. Now 120ms.
- The highlight glided 280ms to wherever a click landed, so crossing
twenty prompts read as the bar flying off across the rail. A click now
owns the highlight until its scroll settles: no glide, and the scroll
no longer walks the highlight through every prompt it passes.
- The first click into a session did nothing until the reader scrolled by
hand. See below.
That last one is a collision between Astryx's auto-follow lock and the
progressive transcript mount, and neither side is wrong on its own.
`useChatStreamScroll` unlocks on a scroll up, detected by comparing
scrollTop between events — but it ignores any scroll event that arrives
with a changed scrollHeight or offsetHeight, because Chrome fires those
when content resizes and they are not the reader moving. A jump into an
unmounted turn mounts it and the fill that follows changes scrollHeight
for several frames, so the jump's own scroll is invisible to the lock: it
stays on, and `scrollIfLocked` pulls the transcript back to the bottom.
Only a wheel gesture broke it, which takes a separate path in Astryx.
`holdJumpDestination` re-aims at the target on each height change until
the fill stops. The last of those scrolls lands with a stable height,
which is the one the lock finally reads as a scroll up. Measured on the
30-prompt fixture: clicking the first tick went to scrollTop 7042 (the
bottom) and now goes to 24 and holds.
The fixture grows from 8 prompts to 30 because the progressive mount's
initial window is 10 — at 8 the head of the transcript is already mounted
and the jump-into-unmounted-turns path never runs at all.
Coverage note: the e2e case for the first click is an end-to-end check,
not a guard. Whether the lock wins depends on which frame the fill lands
on relative to a smooth scroll still in flight, and it goes green against
the unfixed renderer often enough to be worthless as one. The guard is
the `holdJumpDestination` unit test, which drives the frames itself.
* fix(ui): own a rail jump through the mount instead of racing it
Review of #2923 found the jump's ownership bound to a clock rather than to
the navigation, and the e2e case that was supposed to guard it asserting
almost nothing. Both hold.
Jump ownership:
- A second click during a jump only replaced the target; the first click's
700ms timer still governed, and could clear the second jump mid-flight.
Each click now carries its own sequence and starts its own hold.
- The fixed window is gone. A hold runs until the progressive mount reports
the transcript filled AND nothing has moved for a few frames, so a long
transcript is never released mid-fill, and it ends the moment the reader
touches the transcript (wheel, touch, pointer, key) rather than outliving
their interest in it.
Chasing the "just release auto-follow" direction the review preferred found
that ChatLayout publishes no such seam, so this adds one — `unlockAutoFollow`
on `ChatLayoutContextValue`, exposing the scroll hook's existing `unlock`
(patch hunk + patches/README entry). It is necessary and it is not
sufficient, which the earlier framing got wrong:
- Astryx re-locks on any `scrollend` that settles near the bottom, and a
session that opens at the bottom produces exactly that while the mount is
still catching up. Releasing once at the click is undone before the jump
goes anywhere — traced: released at the click, landed at 154ms, dragged
back to the bottom by 166ms. The release is now re-asserted for the life
of the hold.
- Auto-follow is not the only thing moving the transcript. The progressive
mount's own scroll compensation holds the reader's position across each
fill step, and mounting the turn a jump asked for IS a fill step, so it
lands after the jump and restores the position the jump just left. That
one no seam can fix; it is what the hold is for.
Jumps also scroll instantly now, whatever the app's scroll-motion policy
says. A jump is a teleport the reader asked for, and an animated one does
not survive this surface: traced on the 30-prompt fixture, the smooth scroll
was cancelled by the mount's compensation and by the follow spring and
stalled two pixels from where it started.
Coverage:
- The first-click e2e case named the wrong turn (`[data-turn-id]` is the
first MOUNTED turn, whose top is already negative at the opening scroll
position, so an upper-bound-only check passed without the jump doing
anything). It now names `turn-prompt-rail-1`, bounds it on both sides, and
asserts that tick's `aria-current`.
- `emulateMedia` could not put that case on the production scroll path:
`resolveScrollMotionBehavior` collapses motion for ANY fixture, keyed on
`data-maka-e2e-fixture` rather than on the media query. Fixtures can now
ask for a behavior back (`scrollMotion`, per launch — it costs seconds of
settling per window, so only the case that needs it pays), with unit
coverage for the precedence: a fixture request never outranks a stated
preference for less motion.
- `holdJumpDestination`'s unit tests grew the two cases its rewrite is
about: it must not settle while the transcript is still filling, and it
must hand the transcript back the moment the reader touches it.
Verified 5/5 on the smooth-scroll fixture, where the previous revision lost
1 in 4. `quote-selection.spec.ts` flakes on this machine (1 in 4) at
upstream/main as well, unchanged by this branch.
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
claiming a guard that does not exist — the worst state to leave an
assumption in, since the next person reads the citation and stops looking.
Restore the two assertions the comment is actually about, plus the ladder
order #2348 exists to protect, in ONE file. Split across two they each
stayed green against the half they could not see, which is how the
stylesheet half got deleted without anything noticing:
- the ladder is declared on the --space-* scale in strictly increasing
order (retuning a rung is a design call and stays green; list gaps
meeting block gaps fails)
- every rung declared is actually spent by a rule, so the ladder cannot
become decoration while literals do the spacing
- the runtime hooks the selectors need are really emitted by MarkdownBody
— data-maka-contract, data-density, astryx-markdown-heading, data-level,
astryx-list-item. These names have a single upstream owner in Astryx and
appear in Maka only inside selectors, so a rename kills every rule at
once with nothing failing
- compact markdown still has exactly one caller, which is the bet the
comment names: the table carries heading TYPOGRAPHY on a density key,
and Astryx's own RFC says density must not do that
Why these are not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions and CSS-structure suites that
asserted how a rule was written. Nothing here asserts an appearance or a
rule's shape: three of the four are relations that must survive any
retune, and the fourth renders the real component and checks the join
between Astryx's output and Maka's selectors. Deliberately left unpinned,
because they ARE how the table is written: the adjacent-sibling gap form,
the `hr` rung, the ListItem padding reset, and the two heading size tiers.
The stylesheet-reading half no longer revives the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read; it
reads packages/ui/src/styles.css directly, in the package that owns it.
Seven mutations, each verified to fail the assertion that describes it:
inverting the ladder; taking a rung off the --space-* grid; hardcoding a
rung's usage; hardcoding it while naming the rung in a comment (this is
why comments are stripped first); renaming the contract wrapper; cutting
density off before it reaches Astryx; and adding a second compact caller.
Also state in the comment what the contract does and does not hold, so
the next reader does not have to open the test to find out.
Generated-by: Claude Code
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
#2462 and its stylesheet-side sibling in #2425, so the comment has been
citing a guard that does not exist — the worst state to leave an assumption
in, since the next person reads the citation and stops looking.
Restore only what cannot be noticed without a test. Both failure modes here
are silent: no error, no failing check, just spacing that quietly stops
being what the table declares, because a CSS selector that matches nothing
never complains.
- The table selects entirely on DOM Astryx generates at runtime —
data-density, astryx-markdown-heading + data-level, astryx-list-item.
Those names have one upstream owner and appear in Maka only inside
selectors, so a rename kills every rule at once. Astryx is bumped
regularly (0.4.0 in #2983, 0.4.3 in flight, plus the Dependabot minor
group), so this is a recurring event, not a hypothetical.
- Astryx's ListItem carries CONTROL row padding that `density` cannot reach
from outside. That padding is what inverted the ladder in #2348 — list
items ~10px apart against 4px paragraphs — and one rule neutralizes it.
Lose the rule and the original defect returns, silently.
- Plus the assumption the comment actually names: compact markdown still
has exactly one caller. The table carries heading TYPOGRAPHY on a density
key, which Astryx's own RFC says density must not do, so the key is
honest only while `compact` and "transcript" are the same set.
Deliberately NOT pinned: the ladder's declared values and their order. A
first draft asserted them, and it was the wrong instinct — a reversed
ladder has to be typed on purpose into four adjacent lines under a comment
explaining the order, and it is visible the moment anyone looks at a
transcript. It also would not have caught the defect it claimed to guard:
#2348's inversion came from the ListItem padding above, which a test
reading only the declared variables cannot see. Same for the
adjacent-sibling gap form, the `hr` rung and the two heading size tiers —
those are how the table is written, not what it promises.
Why this is not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions, and CSS-structure suites that
asserted how a rule was written. Two of the three assertions here render
the real component and check the join between Astryx's output and Maka's
selectors; the third asserts an outcome the table promises, not a syntax.
The stylesheet-reading half does not revive the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read — it
reads packages/ui/src/styles.css directly, in the package that owns it.
Six mutations, each verified to fail the assertion that describes it:
deleting the ListItem padding reset; un-zeroing it; un-zeroing it while
naming the reset in a comment (this is why comments are stripped first);
renaming the contract wrapper; cutting density off before it reaches
Astryx; and adding a second compact caller.
Generated-by: Claude Code
Sign up for freeto 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.

2 participants

@jackwener@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
test: delete low-value and redundant suite files by jackwener · Pull Request #2462 · apache/maka · GitHub
Skip to content

test: delete low-value and redundant suite files - #2462

Merged
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests
Aug 8, 2026
Merged

test: delete low-value and redundant suite files#2462
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests

Conversation

@jackwener

@jackwenerjackwener commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

Delete 102 low-value or redundant source test files across Desktop, UI, Runtime, Headless, CLI, and Core, reducing the source test-file surface from 988 to 886 (-10.3%).

This branch is now based on the current main. The cleanup deliberately keeps tests that are the only guard for an active dependency patch or a security-sensitive boundary.

Removed

  • Desktop markup, copy, presentation, chrome, and overlapping geometry/workbar coverage
  • UI localization/copy panels, quiet chrome, empty states, and presentation pins
  • Runtime thin wire and duplicate provider helper coverage
  • Headless duplicate A/B, toolchain, and prompt-summary coverage
  • CLI layout/completion coverage and Core copy/locale helpers

Kept after review

  • Active dependency-patch guards:
    • AI SDK tool-call index tracking
    • Astryx List accessible-name forwarding
    • Astryx new-message indicator behavior
  • Security-sensitive boundaries:
    • split-chunk tool-output secret redaction
    • permission escalation confirmation and persistence
    • task-ledger prompt injection/redaction wiring
    • Responses API store: false
    • session-title internal-instruction stripping
  • Root authority, credentials, storage safety, primary runtime/session/tool suites, and fail-safe CI planning

Additional change

  • CLI process-contract tests ignore Node's ExperimentalWarning: SQLite stderr noise while continuing to reject other stderr output.

Validation

  • git diff --check origin/main...HEAD
  • npm run format:check
  • npm test — all workspace tests passed after synchronizing with current main

GitHub CI is running on the updated head.

jackwenerand others added 8 commits August 8, 2026 01:38
Remove 122 source test files (~12% of packages/apps test files) that were
markup/copy contracts, overlapping e2e chrome, duplicate UI presentation
pins, and low-signal headless AB/toolchain matrix cases. Keep safety,
authority, and primary product unit paths.
Also tolerate Node SQLite ExperimentalWarning in CLI run process contracts
so stderr-empty assertions are not flaky on modern Node.
Bring back e2e journeys, headless matrix/export, catalog contract, AiSdkFlow,
and subscription non-leak coverage that are sole or primary gates. Keep the
remaining ~11.6% file-level reduction of true low-value markup/chrome/matrix
duplicates.
Keep the sole PROVIDER_REGISTRY×models.dev sync sweep and the provider
catalog tab/OAuth/slug contract. These are not markup-copy pins and have no
thinner remaining suite.
…lue-tests
# Conflicts:
#	apps/desktop/e2e/new-messages-indicator.spec.ts
#	apps/desktop/e2e/prompt-rail.spec.ts
#	packages/ui/src/__tests__/conversation-localization.test.tsx
@Astro-Han
Astro-Han merged commit 2f5c75b into mainAug 8, 2026
21 of 23 checks passed
@Astro-Han
Astro-Han deleted the cleanup/delete-low-value-tests branch August 8, 2026 03:28
UncertaintyDeterminesYou4ndMe pushed a commit to UncertaintyDeterminesYou4ndMe/maka-agent that referenced this pull request Aug 8, 2026
Part of apache#2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
apache#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Astro-Han pushed a commit that referenced this pull request Aug 8, 2026
Part of #2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Co-authored-by: 陆逊 <211125649+UncertaintyDeterminesYou4ndMe@users.noreply.github.com>
ARE404 added a commit to ARE404/maka-agent that referenced this pull request Aug 13, 2026
apache#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— apache#2161 pinned it against a containing block as tall as the conversation,
apache#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in apache#2462, and the multi-prompt fixtures it ran on in apache#2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
Astro-Han pushed a commit that referenced this pull request Aug 13, 2026
…2923)
* fix(ui): give the prompt rail's tick bars a box again
#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— #2161 pinned it against a containing block as tall as the conversation,
#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in #2462, and the multi-prompt fixtures it ran on in #2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
* fix(ui): make the prompt rail's hover and jump behave
Four things the rail got wrong once it was visible again, found by using it:
- A 4px gap between ticks was a band where the pointer was over the rail
and over no tick, so the dock-style hover falloff dropped out and picked
up again every few pixels of travel. The rail's `gap` moves into the
ticks' own `padding-block`: same pitch, hit boxes now tile.
- The hover preview waited 300ms before opening — Astryx's HoverCard
default, meant for a pointer crossing a wide row on its way somewhere
else. A tick is 22px of rail that nothing is on the way to, and the wait
is the one part of this hover with no motion in it. Now 120ms.
- The highlight glided 280ms to wherever a click landed, so crossing
twenty prompts read as the bar flying off across the rail. A click now
owns the highlight until its scroll settles: no glide, and the scroll
no longer walks the highlight through every prompt it passes.
- The first click into a session did nothing until the reader scrolled by
hand. See below.
That last one is a collision between Astryx's auto-follow lock and the
progressive transcript mount, and neither side is wrong on its own.
`useChatStreamScroll` unlocks on a scroll up, detected by comparing
scrollTop between events — but it ignores any scroll event that arrives
with a changed scrollHeight or offsetHeight, because Chrome fires those
when content resizes and they are not the reader moving. A jump into an
unmounted turn mounts it and the fill that follows changes scrollHeight
for several frames, so the jump's own scroll is invisible to the lock: it
stays on, and `scrollIfLocked` pulls the transcript back to the bottom.
Only a wheel gesture broke it, which takes a separate path in Astryx.
`holdJumpDestination` re-aims at the target on each height change until
the fill stops. The last of those scrolls lands with a stable height,
which is the one the lock finally reads as a scroll up. Measured on the
30-prompt fixture: clicking the first tick went to scrollTop 7042 (the
bottom) and now goes to 24 and holds.
The fixture grows from 8 prompts to 30 because the progressive mount's
initial window is 10 — at 8 the head of the transcript is already mounted
and the jump-into-unmounted-turns path never runs at all.
Coverage note: the e2e case for the first click is an end-to-end check,
not a guard. Whether the lock wins depends on which frame the fill lands
on relative to a smooth scroll still in flight, and it goes green against
the unfixed renderer often enough to be worthless as one. The guard is
the `holdJumpDestination` unit test, which drives the frames itself.
* fix(ui): own a rail jump through the mount instead of racing it
Review of #2923 found the jump's ownership bound to a clock rather than to
the navigation, and the e2e case that was supposed to guard it asserting
almost nothing. Both hold.
Jump ownership:
- A second click during a jump only replaced the target; the first click's
700ms timer still governed, and could clear the second jump mid-flight.
Each click now carries its own sequence and starts its own hold.
- The fixed window is gone. A hold runs until the progressive mount reports
the transcript filled AND nothing has moved for a few frames, so a long
transcript is never released mid-fill, and it ends the moment the reader
touches the transcript (wheel, touch, pointer, key) rather than outliving
their interest in it.
Chasing the "just release auto-follow" direction the review preferred found
that ChatLayout publishes no such seam, so this adds one — `unlockAutoFollow`
on `ChatLayoutContextValue`, exposing the scroll hook's existing `unlock`
(patch hunk + patches/README entry). It is necessary and it is not
sufficient, which the earlier framing got wrong:
- Astryx re-locks on any `scrollend` that settles near the bottom, and a
session that opens at the bottom produces exactly that while the mount is
still catching up. Releasing once at the click is undone before the jump
goes anywhere — traced: released at the click, landed at 154ms, dragged
back to the bottom by 166ms. The release is now re-asserted for the life
of the hold.
- Auto-follow is not the only thing moving the transcript. The progressive
mount's own scroll compensation holds the reader's position across each
fill step, and mounting the turn a jump asked for IS a fill step, so it
lands after the jump and restores the position the jump just left. That
one no seam can fix; it is what the hold is for.
Jumps also scroll instantly now, whatever the app's scroll-motion policy
says. A jump is a teleport the reader asked for, and an animated one does
not survive this surface: traced on the 30-prompt fixture, the smooth scroll
was cancelled by the mount's compensation and by the follow spring and
stalled two pixels from where it started.
Coverage:
- The first-click e2e case named the wrong turn (`[data-turn-id]` is the
first MOUNTED turn, whose top is already negative at the opening scroll
position, so an upper-bound-only check passed without the jump doing
anything). It now names `turn-prompt-rail-1`, bounds it on both sides, and
asserts that tick's `aria-current`.
- `emulateMedia` could not put that case on the production scroll path:
`resolveScrollMotionBehavior` collapses motion for ANY fixture, keyed on
`data-maka-e2e-fixture` rather than on the media query. Fixtures can now
ask for a behavior back (`scrollMotion`, per launch — it costs seconds of
settling per window, so only the case that needs it pays), with unit
coverage for the precedence: a fixture request never outranks a stated
preference for less motion.
- `holdJumpDestination`'s unit tests grew the two cases its rewrite is
about: it must not settle while the transcript is still filling, and it
must hand the transcript back the moment the reader touches it.
Verified 5/5 on the smooth-scroll fixture, where the previous revision lost
1 in 4. `quote-selection.spec.ts` flakes on this machine (1 in 4) at
upstream/main as well, unchanged by this branch.
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
claiming a guard that does not exist — the worst state to leave an
assumption in, since the next person reads the citation and stops looking.
Restore the two assertions the comment is actually about, plus the ladder
order #2348 exists to protect, in ONE file. Split across two they each
stayed green against the half they could not see, which is how the
stylesheet half got deleted without anything noticing:
- the ladder is declared on the --space-* scale in strictly increasing
order (retuning a rung is a design call and stays green; list gaps
meeting block gaps fails)
- every rung declared is actually spent by a rule, so the ladder cannot
become decoration while literals do the spacing
- the runtime hooks the selectors need are really emitted by MarkdownBody
— data-maka-contract, data-density, astryx-markdown-heading, data-level,
astryx-list-item. These names have a single upstream owner in Astryx and
appear in Maka only inside selectors, so a rename kills every rule at
once with nothing failing
- compact markdown still has exactly one caller, which is the bet the
comment names: the table carries heading TYPOGRAPHY on a density key,
and Astryx's own RFC says density must not do that
Why these are not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions and CSS-structure suites that
asserted how a rule was written. Nothing here asserts an appearance or a
rule's shape: three of the four are relations that must survive any
retune, and the fourth renders the real component and checks the join
between Astryx's output and Maka's selectors. Deliberately left unpinned,
because they ARE how the table is written: the adjacent-sibling gap form,
the `hr` rung, the ListItem padding reset, and the two heading size tiers.
The stylesheet-reading half no longer revives the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read; it
reads packages/ui/src/styles.css directly, in the package that owns it.
Seven mutations, each verified to fail the assertion that describes it:
inverting the ladder; taking a rung off the --space-* grid; hardcoding a
rung's usage; hardcoding it while naming the rung in a comment (this is
why comments are stripped first); renaming the contract wrapper; cutting
density off before it reaches Astryx; and adding a second compact caller.
Also state in the comment what the contract does and does not hold, so
the next reader does not have to open the test to find out.
Generated-by: Claude Code
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
#2462 and its stylesheet-side sibling in #2425, so the comment has been
citing a guard that does not exist — the worst state to leave an assumption
in, since the next person reads the citation and stops looking.
Restore only what cannot be noticed without a test. Both failure modes here
are silent: no error, no failing check, just spacing that quietly stops
being what the table declares, because a CSS selector that matches nothing
never complains.
- The table selects entirely on DOM Astryx generates at runtime —
data-density, astryx-markdown-heading + data-level, astryx-list-item.
Those names have one upstream owner and appear in Maka only inside
selectors, so a rename kills every rule at once. Astryx is bumped
regularly (0.4.0 in #2983, 0.4.3 in flight, plus the Dependabot minor
group), so this is a recurring event, not a hypothetical.
- Astryx's ListItem carries CONTROL row padding that `density` cannot reach
from outside. That padding is what inverted the ladder in #2348 — list
items ~10px apart against 4px paragraphs — and one rule neutralizes it.
Lose the rule and the original defect returns, silently.
- Plus the assumption the comment actually names: compact markdown still
has exactly one caller. The table carries heading TYPOGRAPHY on a density
key, which Astryx's own RFC says density must not do, so the key is
honest only while `compact` and "transcript" are the same set.
Deliberately NOT pinned: the ladder's declared values and their order. A
first draft asserted them, and it was the wrong instinct — a reversed
ladder has to be typed on purpose into four adjacent lines under a comment
explaining the order, and it is visible the moment anyone looks at a
transcript. It also would not have caught the defect it claimed to guard:
#2348's inversion came from the ListItem padding above, which a test
reading only the declared variables cannot see. Same for the
adjacent-sibling gap form, the `hr` rung and the two heading size tiers —
those are how the table is written, not what it promises.
Why this is not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions, and CSS-structure suites that
asserted how a rule was written. Two of the three assertions here render
the real component and check the join between Astryx's output and Maka's
selectors; the third asserts an outcome the table promises, not a syntax.
The stylesheet-reading half does not revive the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read — it
reads packages/ui/src/styles.css directly, in the package that owns it.
Six mutations, each verified to fail the assertion that describes it:
deleting the ListItem padding reset; un-zeroing it; un-zeroing it while
naming the reset in a comment (this is why comments are stripped first);
renaming the contract wrapper; cutting density off before it reaches
Astryx; and adding a second compact caller.
Generated-by: Claude Code
Sign up for freeto 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.

2 participants

@jackwener@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' test: delete low-value and redundant suite files by jackwener · Pull Request #2462 · apache/maka · GitHub
Skip to content

test: delete low-value and redundant suite files - #2462

Merged
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests
Aug 8, 2026
Merged

test: delete low-value and redundant suite files#2462
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests

Conversation

@jackwener

@jackwenerjackwener commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

Delete 102 low-value or redundant source test files across Desktop, UI, Runtime, Headless, CLI, and Core, reducing the source test-file surface from 988 to 886 (-10.3%).

This branch is now based on the current main. The cleanup deliberately keeps tests that are the only guard for an active dependency patch or a security-sensitive boundary.

Removed

  • Desktop markup, copy, presentation, chrome, and overlapping geometry/workbar coverage
  • UI localization/copy panels, quiet chrome, empty states, and presentation pins
  • Runtime thin wire and duplicate provider helper coverage
  • Headless duplicate A/B, toolchain, and prompt-summary coverage
  • CLI layout/completion coverage and Core copy/locale helpers

Kept after review

  • Active dependency-patch guards:
    • AI SDK tool-call index tracking
    • Astryx List accessible-name forwarding
    • Astryx new-message indicator behavior
  • Security-sensitive boundaries:
    • split-chunk tool-output secret redaction
    • permission escalation confirmation and persistence
    • task-ledger prompt injection/redaction wiring
    • Responses API store: false
    • session-title internal-instruction stripping
  • Root authority, credentials, storage safety, primary runtime/session/tool suites, and fail-safe CI planning

Additional change

  • CLI process-contract tests ignore Node's ExperimentalWarning: SQLite stderr noise while continuing to reject other stderr output.

Validation

  • git diff --check origin/main...HEAD
  • npm run format:check
  • npm test — all workspace tests passed after synchronizing with current main

GitHub CI is running on the updated head.

jackwenerand others added 8 commits August 8, 2026 01:38
Remove 122 source test files (~12% of packages/apps test files) that were
markup/copy contracts, overlapping e2e chrome, duplicate UI presentation
pins, and low-signal headless AB/toolchain matrix cases. Keep safety,
authority, and primary product unit paths.
Also tolerate Node SQLite ExperimentalWarning in CLI run process contracts
so stderr-empty assertions are not flaky on modern Node.
Bring back e2e journeys, headless matrix/export, catalog contract, AiSdkFlow,
and subscription non-leak coverage that are sole or primary gates. Keep the
remaining ~11.6% file-level reduction of true low-value markup/chrome/matrix
duplicates.
Keep the sole PROVIDER_REGISTRY×models.dev sync sweep and the provider
catalog tab/OAuth/slug contract. These are not markup-copy pins and have no
thinner remaining suite.
…lue-tests
# Conflicts:
#	apps/desktop/e2e/new-messages-indicator.spec.ts
#	apps/desktop/e2e/prompt-rail.spec.ts
#	packages/ui/src/__tests__/conversation-localization.test.tsx
@Astro-Han
Astro-Han merged commit 2f5c75b into mainAug 8, 2026
21 of 23 checks passed
@Astro-Han
Astro-Han deleted the cleanup/delete-low-value-tests branch August 8, 2026 03:28
UncertaintyDeterminesYou4ndMe pushed a commit to UncertaintyDeterminesYou4ndMe/maka-agent that referenced this pull request Aug 8, 2026
Part of apache#2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
apache#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Astro-Han pushed a commit that referenced this pull request Aug 8, 2026
Part of #2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Co-authored-by: 陆逊 <211125649+UncertaintyDeterminesYou4ndMe@users.noreply.github.com>
ARE404 added a commit to ARE404/maka-agent that referenced this pull request Aug 13, 2026
apache#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— apache#2161 pinned it against a containing block as tall as the conversation,
apache#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in apache#2462, and the multi-prompt fixtures it ran on in apache#2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
Astro-Han pushed a commit that referenced this pull request Aug 13, 2026
…2923)
* fix(ui): give the prompt rail's tick bars a box again
#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— #2161 pinned it against a containing block as tall as the conversation,
#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in #2462, and the multi-prompt fixtures it ran on in #2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
* fix(ui): make the prompt rail's hover and jump behave
Four things the rail got wrong once it was visible again, found by using it:
- A 4px gap between ticks was a band where the pointer was over the rail
and over no tick, so the dock-style hover falloff dropped out and picked
up again every few pixels of travel. The rail's `gap` moves into the
ticks' own `padding-block`: same pitch, hit boxes now tile.
- The hover preview waited 300ms before opening — Astryx's HoverCard
default, meant for a pointer crossing a wide row on its way somewhere
else. A tick is 22px of rail that nothing is on the way to, and the wait
is the one part of this hover with no motion in it. Now 120ms.
- The highlight glided 280ms to wherever a click landed, so crossing
twenty prompts read as the bar flying off across the rail. A click now
owns the highlight until its scroll settles: no glide, and the scroll
no longer walks the highlight through every prompt it passes.
- The first click into a session did nothing until the reader scrolled by
hand. See below.
That last one is a collision between Astryx's auto-follow lock and the
progressive transcript mount, and neither side is wrong on its own.
`useChatStreamScroll` unlocks on a scroll up, detected by comparing
scrollTop between events — but it ignores any scroll event that arrives
with a changed scrollHeight or offsetHeight, because Chrome fires those
when content resizes and they are not the reader moving. A jump into an
unmounted turn mounts it and the fill that follows changes scrollHeight
for several frames, so the jump's own scroll is invisible to the lock: it
stays on, and `scrollIfLocked` pulls the transcript back to the bottom.
Only a wheel gesture broke it, which takes a separate path in Astryx.
`holdJumpDestination` re-aims at the target on each height change until
the fill stops. The last of those scrolls lands with a stable height,
which is the one the lock finally reads as a scroll up. Measured on the
30-prompt fixture: clicking the first tick went to scrollTop 7042 (the
bottom) and now goes to 24 and holds.
The fixture grows from 8 prompts to 30 because the progressive mount's
initial window is 10 — at 8 the head of the transcript is already mounted
and the jump-into-unmounted-turns path never runs at all.
Coverage note: the e2e case for the first click is an end-to-end check,
not a guard. Whether the lock wins depends on which frame the fill lands
on relative to a smooth scroll still in flight, and it goes green against
the unfixed renderer often enough to be worthless as one. The guard is
the `holdJumpDestination` unit test, which drives the frames itself.
* fix(ui): own a rail jump through the mount instead of racing it
Review of #2923 found the jump's ownership bound to a clock rather than to
the navigation, and the e2e case that was supposed to guard it asserting
almost nothing. Both hold.
Jump ownership:
- A second click during a jump only replaced the target; the first click's
700ms timer still governed, and could clear the second jump mid-flight.
Each click now carries its own sequence and starts its own hold.
- The fixed window is gone. A hold runs until the progressive mount reports
the transcript filled AND nothing has moved for a few frames, so a long
transcript is never released mid-fill, and it ends the moment the reader
touches the transcript (wheel, touch, pointer, key) rather than outliving
their interest in it.
Chasing the "just release auto-follow" direction the review preferred found
that ChatLayout publishes no such seam, so this adds one — `unlockAutoFollow`
on `ChatLayoutContextValue`, exposing the scroll hook's existing `unlock`
(patch hunk + patches/README entry). It is necessary and it is not
sufficient, which the earlier framing got wrong:
- Astryx re-locks on any `scrollend` that settles near the bottom, and a
session that opens at the bottom produces exactly that while the mount is
still catching up. Releasing once at the click is undone before the jump
goes anywhere — traced: released at the click, landed at 154ms, dragged
back to the bottom by 166ms. The release is now re-asserted for the life
of the hold.
- Auto-follow is not the only thing moving the transcript. The progressive
mount's own scroll compensation holds the reader's position across each
fill step, and mounting the turn a jump asked for IS a fill step, so it
lands after the jump and restores the position the jump just left. That
one no seam can fix; it is what the hold is for.
Jumps also scroll instantly now, whatever the app's scroll-motion policy
says. A jump is a teleport the reader asked for, and an animated one does
not survive this surface: traced on the 30-prompt fixture, the smooth scroll
was cancelled by the mount's compensation and by the follow spring and
stalled two pixels from where it started.
Coverage:
- The first-click e2e case named the wrong turn (`[data-turn-id]` is the
first MOUNTED turn, whose top is already negative at the opening scroll
position, so an upper-bound-only check passed without the jump doing
anything). It now names `turn-prompt-rail-1`, bounds it on both sides, and
asserts that tick's `aria-current`.
- `emulateMedia` could not put that case on the production scroll path:
`resolveScrollMotionBehavior` collapses motion for ANY fixture, keyed on
`data-maka-e2e-fixture` rather than on the media query. Fixtures can now
ask for a behavior back (`scrollMotion`, per launch — it costs seconds of
settling per window, so only the case that needs it pays), with unit
coverage for the precedence: a fixture request never outranks a stated
preference for less motion.
- `holdJumpDestination`'s unit tests grew the two cases its rewrite is
about: it must not settle while the transcript is still filling, and it
must hand the transcript back the moment the reader touches it.
Verified 5/5 on the smooth-scroll fixture, where the previous revision lost
1 in 4. `quote-selection.spec.ts` flakes on this machine (1 in 4) at
upstream/main as well, unchanged by this branch.
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
claiming a guard that does not exist — the worst state to leave an
assumption in, since the next person reads the citation and stops looking.
Restore the two assertions the comment is actually about, plus the ladder
order #2348 exists to protect, in ONE file. Split across two they each
stayed green against the half they could not see, which is how the
stylesheet half got deleted without anything noticing:
- the ladder is declared on the --space-* scale in strictly increasing
order (retuning a rung is a design call and stays green; list gaps
meeting block gaps fails)
- every rung declared is actually spent by a rule, so the ladder cannot
become decoration while literals do the spacing
- the runtime hooks the selectors need are really emitted by MarkdownBody
— data-maka-contract, data-density, astryx-markdown-heading, data-level,
astryx-list-item. These names have a single upstream owner in Astryx and
appear in Maka only inside selectors, so a rename kills every rule at
once with nothing failing
- compact markdown still has exactly one caller, which is the bet the
comment names: the table carries heading TYPOGRAPHY on a density key,
and Astryx's own RFC says density must not do that
Why these are not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions and CSS-structure suites that
asserted how a rule was written. Nothing here asserts an appearance or a
rule's shape: three of the four are relations that must survive any
retune, and the fourth renders the real component and checks the join
between Astryx's output and Maka's selectors. Deliberately left unpinned,
because they ARE how the table is written: the adjacent-sibling gap form,
the `hr` rung, the ListItem padding reset, and the two heading size tiers.
The stylesheet-reading half no longer revives the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read; it
reads packages/ui/src/styles.css directly, in the package that owns it.
Seven mutations, each verified to fail the assertion that describes it:
inverting the ladder; taking a rung off the --space-* grid; hardcoding a
rung's usage; hardcoding it while naming the rung in a comment (this is
why comments are stripped first); renaming the contract wrapper; cutting
density off before it reaches Astryx; and adding a second compact caller.
Also state in the comment what the contract does and does not hold, so
the next reader does not have to open the test to find out.
Generated-by: Claude Code
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
#2462 and its stylesheet-side sibling in #2425, so the comment has been
citing a guard that does not exist — the worst state to leave an assumption
in, since the next person reads the citation and stops looking.
Restore only what cannot be noticed without a test. Both failure modes here
are silent: no error, no failing check, just spacing that quietly stops
being what the table declares, because a CSS selector that matches nothing
never complains.
- The table selects entirely on DOM Astryx generates at runtime —
data-density, astryx-markdown-heading + data-level, astryx-list-item.
Those names have one upstream owner and appear in Maka only inside
selectors, so a rename kills every rule at once. Astryx is bumped
regularly (0.4.0 in #2983, 0.4.3 in flight, plus the Dependabot minor
group), so this is a recurring event, not a hypothetical.
- Astryx's ListItem carries CONTROL row padding that `density` cannot reach
from outside. That padding is what inverted the ladder in #2348 — list
items ~10px apart against 4px paragraphs — and one rule neutralizes it.
Lose the rule and the original defect returns, silently.
- Plus the assumption the comment actually names: compact markdown still
has exactly one caller. The table carries heading TYPOGRAPHY on a density
key, which Astryx's own RFC says density must not do, so the key is
honest only while `compact` and "transcript" are the same set.
Deliberately NOT pinned: the ladder's declared values and their order. A
first draft asserted them, and it was the wrong instinct — a reversed
ladder has to be typed on purpose into four adjacent lines under a comment
explaining the order, and it is visible the moment anyone looks at a
transcript. It also would not have caught the defect it claimed to guard:
#2348's inversion came from the ListItem padding above, which a test
reading only the declared variables cannot see. Same for the
adjacent-sibling gap form, the `hr` rung and the two heading size tiers —
those are how the table is written, not what it promises.
Why this is not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions, and CSS-structure suites that
asserted how a rule was written. Two of the three assertions here render
the real component and check the join between Astryx's output and Maka's
selectors; the third asserts an outcome the table promises, not a syntax.
The stylesheet-reading half does not revive the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read — it
reads packages/ui/src/styles.css directly, in the package that owns it.
Six mutations, each verified to fail the assertion that describes it:
deleting the ListItem padding reset; un-zeroing it; un-zeroing it while
naming the reset in a comment (this is why comments are stripped first);
renaming the contract wrapper; cutting density off before it reaches
Astryx; and adding a second compact caller.
Generated-by: Claude Code
Sign up for freeto 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.

2 participants

@jackwener@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' test: delete low-value and redundant suite files by jackwener · Pull Request #2462 · apache/maka · GitHub
Skip to content

test: delete low-value and redundant suite files - #2462

Merged
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests
Aug 8, 2026
Merged

test: delete low-value and redundant suite files#2462
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests

Conversation

@jackwener

@jackwenerjackwener commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

Delete 102 low-value or redundant source test files across Desktop, UI, Runtime, Headless, CLI, and Core, reducing the source test-file surface from 988 to 886 (-10.3%).

This branch is now based on the current main. The cleanup deliberately keeps tests that are the only guard for an active dependency patch or a security-sensitive boundary.

Removed

  • Desktop markup, copy, presentation, chrome, and overlapping geometry/workbar coverage
  • UI localization/copy panels, quiet chrome, empty states, and presentation pins
  • Runtime thin wire and duplicate provider helper coverage
  • Headless duplicate A/B, toolchain, and prompt-summary coverage
  • CLI layout/completion coverage and Core copy/locale helpers

Kept after review

  • Active dependency-patch guards:
    • AI SDK tool-call index tracking
    • Astryx List accessible-name forwarding
    • Astryx new-message indicator behavior
  • Security-sensitive boundaries:
    • split-chunk tool-output secret redaction
    • permission escalation confirmation and persistence
    • task-ledger prompt injection/redaction wiring
    • Responses API store: false
    • session-title internal-instruction stripping
  • Root authority, credentials, storage safety, primary runtime/session/tool suites, and fail-safe CI planning

Additional change

  • CLI process-contract tests ignore Node's ExperimentalWarning: SQLite stderr noise while continuing to reject other stderr output.

Validation

  • git diff --check origin/main...HEAD
  • npm run format:check
  • npm test — all workspace tests passed after synchronizing with current main

GitHub CI is running on the updated head.

jackwenerand others added 8 commits August 8, 2026 01:38
Remove 122 source test files (~12% of packages/apps test files) that were
markup/copy contracts, overlapping e2e chrome, duplicate UI presentation
pins, and low-signal headless AB/toolchain matrix cases. Keep safety,
authority, and primary product unit paths.
Also tolerate Node SQLite ExperimentalWarning in CLI run process contracts
so stderr-empty assertions are not flaky on modern Node.
Bring back e2e journeys, headless matrix/export, catalog contract, AiSdkFlow,
and subscription non-leak coverage that are sole or primary gates. Keep the
remaining ~11.6% file-level reduction of true low-value markup/chrome/matrix
duplicates.
Keep the sole PROVIDER_REGISTRY×models.dev sync sweep and the provider
catalog tab/OAuth/slug contract. These are not markup-copy pins and have no
thinner remaining suite.
…lue-tests
# Conflicts:
#	apps/desktop/e2e/new-messages-indicator.spec.ts
#	apps/desktop/e2e/prompt-rail.spec.ts
#	packages/ui/src/__tests__/conversation-localization.test.tsx
@Astro-Han
Astro-Han merged commit 2f5c75b into mainAug 8, 2026
21 of 23 checks passed
@Astro-Han
Astro-Han deleted the cleanup/delete-low-value-tests branch August 8, 2026 03:28
UncertaintyDeterminesYou4ndMe pushed a commit to UncertaintyDeterminesYou4ndMe/maka-agent that referenced this pull request Aug 8, 2026
Part of apache#2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
apache#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Astro-Han pushed a commit that referenced this pull request Aug 8, 2026
Part of #2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Co-authored-by: 陆逊 <211125649+UncertaintyDeterminesYou4ndMe@users.noreply.github.com>
ARE404 added a commit to ARE404/maka-agent that referenced this pull request Aug 13, 2026
apache#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— apache#2161 pinned it against a containing block as tall as the conversation,
apache#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in apache#2462, and the multi-prompt fixtures it ran on in apache#2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
Astro-Han pushed a commit that referenced this pull request Aug 13, 2026
…2923)
* fix(ui): give the prompt rail's tick bars a box again
#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— #2161 pinned it against a containing block as tall as the conversation,
#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in #2462, and the multi-prompt fixtures it ran on in #2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
* fix(ui): make the prompt rail's hover and jump behave
Four things the rail got wrong once it was visible again, found by using it:
- A 4px gap between ticks was a band where the pointer was over the rail
and over no tick, so the dock-style hover falloff dropped out and picked
up again every few pixels of travel. The rail's `gap` moves into the
ticks' own `padding-block`: same pitch, hit boxes now tile.
- The hover preview waited 300ms before opening — Astryx's HoverCard
default, meant for a pointer crossing a wide row on its way somewhere
else. A tick is 22px of rail that nothing is on the way to, and the wait
is the one part of this hover with no motion in it. Now 120ms.
- The highlight glided 280ms to wherever a click landed, so crossing
twenty prompts read as the bar flying off across the rail. A click now
owns the highlight until its scroll settles: no glide, and the scroll
no longer walks the highlight through every prompt it passes.
- The first click into a session did nothing until the reader scrolled by
hand. See below.
That last one is a collision between Astryx's auto-follow lock and the
progressive transcript mount, and neither side is wrong on its own.
`useChatStreamScroll` unlocks on a scroll up, detected by comparing
scrollTop between events — but it ignores any scroll event that arrives
with a changed scrollHeight or offsetHeight, because Chrome fires those
when content resizes and they are not the reader moving. A jump into an
unmounted turn mounts it and the fill that follows changes scrollHeight
for several frames, so the jump's own scroll is invisible to the lock: it
stays on, and `scrollIfLocked` pulls the transcript back to the bottom.
Only a wheel gesture broke it, which takes a separate path in Astryx.
`holdJumpDestination` re-aims at the target on each height change until
the fill stops. The last of those scrolls lands with a stable height,
which is the one the lock finally reads as a scroll up. Measured on the
30-prompt fixture: clicking the first tick went to scrollTop 7042 (the
bottom) and now goes to 24 and holds.
The fixture grows from 8 prompts to 30 because the progressive mount's
initial window is 10 — at 8 the head of the transcript is already mounted
and the jump-into-unmounted-turns path never runs at all.
Coverage note: the e2e case for the first click is an end-to-end check,
not a guard. Whether the lock wins depends on which frame the fill lands
on relative to a smooth scroll still in flight, and it goes green against
the unfixed renderer often enough to be worthless as one. The guard is
the `holdJumpDestination` unit test, which drives the frames itself.
* fix(ui): own a rail jump through the mount instead of racing it
Review of #2923 found the jump's ownership bound to a clock rather than to
the navigation, and the e2e case that was supposed to guard it asserting
almost nothing. Both hold.
Jump ownership:
- A second click during a jump only replaced the target; the first click's
700ms timer still governed, and could clear the second jump mid-flight.
Each click now carries its own sequence and starts its own hold.
- The fixed window is gone. A hold runs until the progressive mount reports
the transcript filled AND nothing has moved for a few frames, so a long
transcript is never released mid-fill, and it ends the moment the reader
touches the transcript (wheel, touch, pointer, key) rather than outliving
their interest in it.
Chasing the "just release auto-follow" direction the review preferred found
that ChatLayout publishes no such seam, so this adds one — `unlockAutoFollow`
on `ChatLayoutContextValue`, exposing the scroll hook's existing `unlock`
(patch hunk + patches/README entry). It is necessary and it is not
sufficient, which the earlier framing got wrong:
- Astryx re-locks on any `scrollend` that settles near the bottom, and a
session that opens at the bottom produces exactly that while the mount is
still catching up. Releasing once at the click is undone before the jump
goes anywhere — traced: released at the click, landed at 154ms, dragged
back to the bottom by 166ms. The release is now re-asserted for the life
of the hold.
- Auto-follow is not the only thing moving the transcript. The progressive
mount's own scroll compensation holds the reader's position across each
fill step, and mounting the turn a jump asked for IS a fill step, so it
lands after the jump and restores the position the jump just left. That
one no seam can fix; it is what the hold is for.
Jumps also scroll instantly now, whatever the app's scroll-motion policy
says. A jump is a teleport the reader asked for, and an animated one does
not survive this surface: traced on the 30-prompt fixture, the smooth scroll
was cancelled by the mount's compensation and by the follow spring and
stalled two pixels from where it started.
Coverage:
- The first-click e2e case named the wrong turn (`[data-turn-id]` is the
first MOUNTED turn, whose top is already negative at the opening scroll
position, so an upper-bound-only check passed without the jump doing
anything). It now names `turn-prompt-rail-1`, bounds it on both sides, and
asserts that tick's `aria-current`.
- `emulateMedia` could not put that case on the production scroll path:
`resolveScrollMotionBehavior` collapses motion for ANY fixture, keyed on
`data-maka-e2e-fixture` rather than on the media query. Fixtures can now
ask for a behavior back (`scrollMotion`, per launch — it costs seconds of
settling per window, so only the case that needs it pays), with unit
coverage for the precedence: a fixture request never outranks a stated
preference for less motion.
- `holdJumpDestination`'s unit tests grew the two cases its rewrite is
about: it must not settle while the transcript is still filling, and it
must hand the transcript back the moment the reader touches it.
Verified 5/5 on the smooth-scroll fixture, where the previous revision lost
1 in 4. `quote-selection.spec.ts` flakes on this machine (1 in 4) at
upstream/main as well, unchanged by this branch.
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
claiming a guard that does not exist — the worst state to leave an
assumption in, since the next person reads the citation and stops looking.
Restore the two assertions the comment is actually about, plus the ladder
order #2348 exists to protect, in ONE file. Split across two they each
stayed green against the half they could not see, which is how the
stylesheet half got deleted without anything noticing:
- the ladder is declared on the --space-* scale in strictly increasing
order (retuning a rung is a design call and stays green; list gaps
meeting block gaps fails)
- every rung declared is actually spent by a rule, so the ladder cannot
become decoration while literals do the spacing
- the runtime hooks the selectors need are really emitted by MarkdownBody
— data-maka-contract, data-density, astryx-markdown-heading, data-level,
astryx-list-item. These names have a single upstream owner in Astryx and
appear in Maka only inside selectors, so a rename kills every rule at
once with nothing failing
- compact markdown still has exactly one caller, which is the bet the
comment names: the table carries heading TYPOGRAPHY on a density key,
and Astryx's own RFC says density must not do that
Why these are not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions and CSS-structure suites that
asserted how a rule was written. Nothing here asserts an appearance or a
rule's shape: three of the four are relations that must survive any
retune, and the fourth renders the real component and checks the join
between Astryx's output and Maka's selectors. Deliberately left unpinned,
because they ARE how the table is written: the adjacent-sibling gap form,
the `hr` rung, the ListItem padding reset, and the two heading size tiers.
The stylesheet-reading half no longer revives the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read; it
reads packages/ui/src/styles.css directly, in the package that owns it.
Seven mutations, each verified to fail the assertion that describes it:
inverting the ladder; taking a rung off the --space-* grid; hardcoding a
rung's usage; hardcoding it while naming the rung in a comment (this is
why comments are stripped first); renaming the contract wrapper; cutting
density off before it reaches Astryx; and adding a second compact caller.
Also state in the comment what the contract does and does not hold, so
the next reader does not have to open the test to find out.
Generated-by: Claude Code
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
#2462 and its stylesheet-side sibling in #2425, so the comment has been
citing a guard that does not exist — the worst state to leave an assumption
in, since the next person reads the citation and stops looking.
Restore only what cannot be noticed without a test. Both failure modes here
are silent: no error, no failing check, just spacing that quietly stops
being what the table declares, because a CSS selector that matches nothing
never complains.
- The table selects entirely on DOM Astryx generates at runtime —
data-density, astryx-markdown-heading + data-level, astryx-list-item.
Those names have one upstream owner and appear in Maka only inside
selectors, so a rename kills every rule at once. Astryx is bumped
regularly (0.4.0 in #2983, 0.4.3 in flight, plus the Dependabot minor
group), so this is a recurring event, not a hypothetical.
- Astryx's ListItem carries CONTROL row padding that `density` cannot reach
from outside. That padding is what inverted the ladder in #2348 — list
items ~10px apart against 4px paragraphs — and one rule neutralizes it.
Lose the rule and the original defect returns, silently.
- Plus the assumption the comment actually names: compact markdown still
has exactly one caller. The table carries heading TYPOGRAPHY on a density
key, which Astryx's own RFC says density must not do, so the key is
honest only while `compact` and "transcript" are the same set.
Deliberately NOT pinned: the ladder's declared values and their order. A
first draft asserted them, and it was the wrong instinct — a reversed
ladder has to be typed on purpose into four adjacent lines under a comment
explaining the order, and it is visible the moment anyone looks at a
transcript. It also would not have caught the defect it claimed to guard:
#2348's inversion came from the ListItem padding above, which a test
reading only the declared variables cannot see. Same for the
adjacent-sibling gap form, the `hr` rung and the two heading size tiers —
those are how the table is written, not what it promises.
Why this is not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions, and CSS-structure suites that
asserted how a rule was written. Two of the three assertions here render
the real component and check the join between Astryx's output and Maka's
selectors; the third asserts an outcome the table promises, not a syntax.
The stylesheet-reading half does not revive the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read — it
reads packages/ui/src/styles.css directly, in the package that owns it.
Six mutations, each verified to fail the assertion that describes it:
deleting the ListItem padding reset; un-zeroing it; un-zeroing it while
naming the reset in a comment (this is why comments are stripped first);
renaming the contract wrapper; cutting density off before it reaches
Astryx; and adding a second compact caller.
Generated-by: Claude Code
Sign up for freeto 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.

2 participants

@jackwener@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' test: delete low-value and redundant suite files by jackwener · Pull Request #2462 · apache/maka · GitHub
Skip to content

test: delete low-value and redundant suite files - #2462

Merged
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests
Aug 8, 2026
Merged

test: delete low-value and redundant suite files#2462
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests

Conversation

@jackwener

@jackwenerjackwener commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

Delete 102 low-value or redundant source test files across Desktop, UI, Runtime, Headless, CLI, and Core, reducing the source test-file surface from 988 to 886 (-10.3%).

This branch is now based on the current main. The cleanup deliberately keeps tests that are the only guard for an active dependency patch or a security-sensitive boundary.

Removed

  • Desktop markup, copy, presentation, chrome, and overlapping geometry/workbar coverage
  • UI localization/copy panels, quiet chrome, empty states, and presentation pins
  • Runtime thin wire and duplicate provider helper coverage
  • Headless duplicate A/B, toolchain, and prompt-summary coverage
  • CLI layout/completion coverage and Core copy/locale helpers

Kept after review

  • Active dependency-patch guards:
    • AI SDK tool-call index tracking
    • Astryx List accessible-name forwarding
    • Astryx new-message indicator behavior
  • Security-sensitive boundaries:
    • split-chunk tool-output secret redaction
    • permission escalation confirmation and persistence
    • task-ledger prompt injection/redaction wiring
    • Responses API store: false
    • session-title internal-instruction stripping
  • Root authority, credentials, storage safety, primary runtime/session/tool suites, and fail-safe CI planning

Additional change

  • CLI process-contract tests ignore Node's ExperimentalWarning: SQLite stderr noise while continuing to reject other stderr output.

Validation

  • git diff --check origin/main...HEAD
  • npm run format:check
  • npm test — all workspace tests passed after synchronizing with current main

GitHub CI is running on the updated head.

jackwenerand others added 8 commits August 8, 2026 01:38
Remove 122 source test files (~12% of packages/apps test files) that were
markup/copy contracts, overlapping e2e chrome, duplicate UI presentation
pins, and low-signal headless AB/toolchain matrix cases. Keep safety,
authority, and primary product unit paths.
Also tolerate Node SQLite ExperimentalWarning in CLI run process contracts
so stderr-empty assertions are not flaky on modern Node.
Bring back e2e journeys, headless matrix/export, catalog contract, AiSdkFlow,
and subscription non-leak coverage that are sole or primary gates. Keep the
remaining ~11.6% file-level reduction of true low-value markup/chrome/matrix
duplicates.
Keep the sole PROVIDER_REGISTRY×models.dev sync sweep and the provider
catalog tab/OAuth/slug contract. These are not markup-copy pins and have no
thinner remaining suite.
…lue-tests
# Conflicts:
#	apps/desktop/e2e/new-messages-indicator.spec.ts
#	apps/desktop/e2e/prompt-rail.spec.ts
#	packages/ui/src/__tests__/conversation-localization.test.tsx
@Astro-Han
Astro-Han merged commit 2f5c75b into mainAug 8, 2026
21 of 23 checks passed
@Astro-Han
Astro-Han deleted the cleanup/delete-low-value-tests branch August 8, 2026 03:28
UncertaintyDeterminesYou4ndMe pushed a commit to UncertaintyDeterminesYou4ndMe/maka-agent that referenced this pull request Aug 8, 2026
Part of apache#2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
apache#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Astro-Han pushed a commit that referenced this pull request Aug 8, 2026
Part of #2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Co-authored-by: 陆逊 <211125649+UncertaintyDeterminesYou4ndMe@users.noreply.github.com>
ARE404 added a commit to ARE404/maka-agent that referenced this pull request Aug 13, 2026
apache#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— apache#2161 pinned it against a containing block as tall as the conversation,
apache#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in apache#2462, and the multi-prompt fixtures it ran on in apache#2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
Astro-Han pushed a commit that referenced this pull request Aug 13, 2026
…2923)
* fix(ui): give the prompt rail's tick bars a box again
#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— #2161 pinned it against a containing block as tall as the conversation,
#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in #2462, and the multi-prompt fixtures it ran on in #2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
* fix(ui): make the prompt rail's hover and jump behave
Four things the rail got wrong once it was visible again, found by using it:
- A 4px gap between ticks was a band where the pointer was over the rail
and over no tick, so the dock-style hover falloff dropped out and picked
up again every few pixels of travel. The rail's `gap` moves into the
ticks' own `padding-block`: same pitch, hit boxes now tile.
- The hover preview waited 300ms before opening — Astryx's HoverCard
default, meant for a pointer crossing a wide row on its way somewhere
else. A tick is 22px of rail that nothing is on the way to, and the wait
is the one part of this hover with no motion in it. Now 120ms.
- The highlight glided 280ms to wherever a click landed, so crossing
twenty prompts read as the bar flying off across the rail. A click now
owns the highlight until its scroll settles: no glide, and the scroll
no longer walks the highlight through every prompt it passes.
- The first click into a session did nothing until the reader scrolled by
hand. See below.
That last one is a collision between Astryx's auto-follow lock and the
progressive transcript mount, and neither side is wrong on its own.
`useChatStreamScroll` unlocks on a scroll up, detected by comparing
scrollTop between events — but it ignores any scroll event that arrives
with a changed scrollHeight or offsetHeight, because Chrome fires those
when content resizes and they are not the reader moving. A jump into an
unmounted turn mounts it and the fill that follows changes scrollHeight
for several frames, so the jump's own scroll is invisible to the lock: it
stays on, and `scrollIfLocked` pulls the transcript back to the bottom.
Only a wheel gesture broke it, which takes a separate path in Astryx.
`holdJumpDestination` re-aims at the target on each height change until
the fill stops. The last of those scrolls lands with a stable height,
which is the one the lock finally reads as a scroll up. Measured on the
30-prompt fixture: clicking the first tick went to scrollTop 7042 (the
bottom) and now goes to 24 and holds.
The fixture grows from 8 prompts to 30 because the progressive mount's
initial window is 10 — at 8 the head of the transcript is already mounted
and the jump-into-unmounted-turns path never runs at all.
Coverage note: the e2e case for the first click is an end-to-end check,
not a guard. Whether the lock wins depends on which frame the fill lands
on relative to a smooth scroll still in flight, and it goes green against
the unfixed renderer often enough to be worthless as one. The guard is
the `holdJumpDestination` unit test, which drives the frames itself.
* fix(ui): own a rail jump through the mount instead of racing it
Review of #2923 found the jump's ownership bound to a clock rather than to
the navigation, and the e2e case that was supposed to guard it asserting
almost nothing. Both hold.
Jump ownership:
- A second click during a jump only replaced the target; the first click's
700ms timer still governed, and could clear the second jump mid-flight.
Each click now carries its own sequence and starts its own hold.
- The fixed window is gone. A hold runs until the progressive mount reports
the transcript filled AND nothing has moved for a few frames, so a long
transcript is never released mid-fill, and it ends the moment the reader
touches the transcript (wheel, touch, pointer, key) rather than outliving
their interest in it.
Chasing the "just release auto-follow" direction the review preferred found
that ChatLayout publishes no such seam, so this adds one — `unlockAutoFollow`
on `ChatLayoutContextValue`, exposing the scroll hook's existing `unlock`
(patch hunk + patches/README entry). It is necessary and it is not
sufficient, which the earlier framing got wrong:
- Astryx re-locks on any `scrollend` that settles near the bottom, and a
session that opens at the bottom produces exactly that while the mount is
still catching up. Releasing once at the click is undone before the jump
goes anywhere — traced: released at the click, landed at 154ms, dragged
back to the bottom by 166ms. The release is now re-asserted for the life
of the hold.
- Auto-follow is not the only thing moving the transcript. The progressive
mount's own scroll compensation holds the reader's position across each
fill step, and mounting the turn a jump asked for IS a fill step, so it
lands after the jump and restores the position the jump just left. That
one no seam can fix; it is what the hold is for.
Jumps also scroll instantly now, whatever the app's scroll-motion policy
says. A jump is a teleport the reader asked for, and an animated one does
not survive this surface: traced on the 30-prompt fixture, the smooth scroll
was cancelled by the mount's compensation and by the follow spring and
stalled two pixels from where it started.
Coverage:
- The first-click e2e case named the wrong turn (`[data-turn-id]` is the
first MOUNTED turn, whose top is already negative at the opening scroll
position, so an upper-bound-only check passed without the jump doing
anything). It now names `turn-prompt-rail-1`, bounds it on both sides, and
asserts that tick's `aria-current`.
- `emulateMedia` could not put that case on the production scroll path:
`resolveScrollMotionBehavior` collapses motion for ANY fixture, keyed on
`data-maka-e2e-fixture` rather than on the media query. Fixtures can now
ask for a behavior back (`scrollMotion`, per launch — it costs seconds of
settling per window, so only the case that needs it pays), with unit
coverage for the precedence: a fixture request never outranks a stated
preference for less motion.
- `holdJumpDestination`'s unit tests grew the two cases its rewrite is
about: it must not settle while the transcript is still filling, and it
must hand the transcript back the moment the reader touches it.
Verified 5/5 on the smooth-scroll fixture, where the previous revision lost
1 in 4. `quote-selection.spec.ts` flakes on this machine (1 in 4) at
upstream/main as well, unchanged by this branch.
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
claiming a guard that does not exist — the worst state to leave an
assumption in, since the next person reads the citation and stops looking.
Restore the two assertions the comment is actually about, plus the ladder
order #2348 exists to protect, in ONE file. Split across two they each
stayed green against the half they could not see, which is how the
stylesheet half got deleted without anything noticing:
- the ladder is declared on the --space-* scale in strictly increasing
order (retuning a rung is a design call and stays green; list gaps
meeting block gaps fails)
- every rung declared is actually spent by a rule, so the ladder cannot
become decoration while literals do the spacing
- the runtime hooks the selectors need are really emitted by MarkdownBody
— data-maka-contract, data-density, astryx-markdown-heading, data-level,
astryx-list-item. These names have a single upstream owner in Astryx and
appear in Maka only inside selectors, so a rename kills every rule at
once with nothing failing
- compact markdown still has exactly one caller, which is the bet the
comment names: the table carries heading TYPOGRAPHY on a density key,
and Astryx's own RFC says density must not do that
Why these are not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions and CSS-structure suites that
asserted how a rule was written. Nothing here asserts an appearance or a
rule's shape: three of the four are relations that must survive any
retune, and the fourth renders the real component and checks the join
between Astryx's output and Maka's selectors. Deliberately left unpinned,
because they ARE how the table is written: the adjacent-sibling gap form,
the `hr` rung, the ListItem padding reset, and the two heading size tiers.
The stylesheet-reading half no longer revives the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read; it
reads packages/ui/src/styles.css directly, in the package that owns it.
Seven mutations, each verified to fail the assertion that describes it:
inverting the ladder; taking a rung off the --space-* grid; hardcoding a
rung's usage; hardcoding it while naming the rung in a comment (this is
why comments are stripped first); renaming the contract wrapper; cutting
density off before it reaches Astryx; and adding a second compact caller.
Also state in the comment what the contract does and does not hold, so
the next reader does not have to open the test to find out.
Generated-by: Claude Code
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
#2462 and its stylesheet-side sibling in #2425, so the comment has been
citing a guard that does not exist — the worst state to leave an assumption
in, since the next person reads the citation and stops looking.
Restore only what cannot be noticed without a test. Both failure modes here
are silent: no error, no failing check, just spacing that quietly stops
being what the table declares, because a CSS selector that matches nothing
never complains.
- The table selects entirely on DOM Astryx generates at runtime —
data-density, astryx-markdown-heading + data-level, astryx-list-item.
Those names have one upstream owner and appear in Maka only inside
selectors, so a rename kills every rule at once. Astryx is bumped
regularly (0.4.0 in #2983, 0.4.3 in flight, plus the Dependabot minor
group), so this is a recurring event, not a hypothetical.
- Astryx's ListItem carries CONTROL row padding that `density` cannot reach
from outside. That padding is what inverted the ladder in #2348 — list
items ~10px apart against 4px paragraphs — and one rule neutralizes it.
Lose the rule and the original defect returns, silently.
- Plus the assumption the comment actually names: compact markdown still
has exactly one caller. The table carries heading TYPOGRAPHY on a density
key, which Astryx's own RFC says density must not do, so the key is
honest only while `compact` and "transcript" are the same set.
Deliberately NOT pinned: the ladder's declared values and their order. A
first draft asserted them, and it was the wrong instinct — a reversed
ladder has to be typed on purpose into four adjacent lines under a comment
explaining the order, and it is visible the moment anyone looks at a
transcript. It also would not have caught the defect it claimed to guard:
#2348's inversion came from the ListItem padding above, which a test
reading only the declared variables cannot see. Same for the
adjacent-sibling gap form, the `hr` rung and the two heading size tiers —
those are how the table is written, not what it promises.
Why this is not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions, and CSS-structure suites that
asserted how a rule was written. Two of the three assertions here render
the real component and check the join between Astryx's output and Maka's
selectors; the third asserts an outcome the table promises, not a syntax.
The stylesheet-reading half does not revive the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read — it
reads packages/ui/src/styles.css directly, in the package that owns it.
Six mutations, each verified to fail the assertion that describes it:
deleting the ListItem padding reset; un-zeroing it; un-zeroing it while
naming the reset in a comment (this is why comments are stripped first);
renaming the contract wrapper; cutting density off before it reaches
Astryx; and adding a second compact caller.
Generated-by: Claude Code
Sign up for freeto 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.

2 participants

@jackwener@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' test: delete low-value and redundant suite files by jackwener · Pull Request #2462 · apache/maka · GitHub
Skip to content

test: delete low-value and redundant suite files - #2462

Merged
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests
Aug 8, 2026
Merged

test: delete low-value and redundant suite files#2462
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests

Conversation

@jackwener

@jackwenerjackwener commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

Delete 102 low-value or redundant source test files across Desktop, UI, Runtime, Headless, CLI, and Core, reducing the source test-file surface from 988 to 886 (-10.3%).

This branch is now based on the current main. The cleanup deliberately keeps tests that are the only guard for an active dependency patch or a security-sensitive boundary.

Removed

  • Desktop markup, copy, presentation, chrome, and overlapping geometry/workbar coverage
  • UI localization/copy panels, quiet chrome, empty states, and presentation pins
  • Runtime thin wire and duplicate provider helper coverage
  • Headless duplicate A/B, toolchain, and prompt-summary coverage
  • CLI layout/completion coverage and Core copy/locale helpers

Kept after review

  • Active dependency-patch guards:
    • AI SDK tool-call index tracking
    • Astryx List accessible-name forwarding
    • Astryx new-message indicator behavior
  • Security-sensitive boundaries:
    • split-chunk tool-output secret redaction
    • permission escalation confirmation and persistence
    • task-ledger prompt injection/redaction wiring
    • Responses API store: false
    • session-title internal-instruction stripping
  • Root authority, credentials, storage safety, primary runtime/session/tool suites, and fail-safe CI planning

Additional change

  • CLI process-contract tests ignore Node's ExperimentalWarning: SQLite stderr noise while continuing to reject other stderr output.

Validation

  • git diff --check origin/main...HEAD
  • npm run format:check
  • npm test — all workspace tests passed after synchronizing with current main

GitHub CI is running on the updated head.

jackwenerand others added 8 commits August 8, 2026 01:38
Remove 122 source test files (~12% of packages/apps test files) that were
markup/copy contracts, overlapping e2e chrome, duplicate UI presentation
pins, and low-signal headless AB/toolchain matrix cases. Keep safety,
authority, and primary product unit paths.
Also tolerate Node SQLite ExperimentalWarning in CLI run process contracts
so stderr-empty assertions are not flaky on modern Node.
Bring back e2e journeys, headless matrix/export, catalog contract, AiSdkFlow,
and subscription non-leak coverage that are sole or primary gates. Keep the
remaining ~11.6% file-level reduction of true low-value markup/chrome/matrix
duplicates.
Keep the sole PROVIDER_REGISTRY×models.dev sync sweep and the provider
catalog tab/OAuth/slug contract. These are not markup-copy pins and have no
thinner remaining suite.
…lue-tests
# Conflicts:
#	apps/desktop/e2e/new-messages-indicator.spec.ts
#	apps/desktop/e2e/prompt-rail.spec.ts
#	packages/ui/src/__tests__/conversation-localization.test.tsx
@Astro-Han
Astro-Han merged commit 2f5c75b into mainAug 8, 2026
21 of 23 checks passed
@Astro-Han
Astro-Han deleted the cleanup/delete-low-value-tests branch August 8, 2026 03:28
UncertaintyDeterminesYou4ndMe pushed a commit to UncertaintyDeterminesYou4ndMe/maka-agent that referenced this pull request Aug 8, 2026
Part of apache#2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
apache#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Astro-Han pushed a commit that referenced this pull request Aug 8, 2026
Part of #2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Co-authored-by: 陆逊 <211125649+UncertaintyDeterminesYou4ndMe@users.noreply.github.com>
ARE404 added a commit to ARE404/maka-agent that referenced this pull request Aug 13, 2026
apache#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— apache#2161 pinned it against a containing block as tall as the conversation,
apache#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in apache#2462, and the multi-prompt fixtures it ran on in apache#2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
Astro-Han pushed a commit that referenced this pull request Aug 13, 2026
…2923)
* fix(ui): give the prompt rail's tick bars a box again
#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— #2161 pinned it against a containing block as tall as the conversation,
#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in #2462, and the multi-prompt fixtures it ran on in #2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
* fix(ui): make the prompt rail's hover and jump behave
Four things the rail got wrong once it was visible again, found by using it:
- A 4px gap between ticks was a band where the pointer was over the rail
and over no tick, so the dock-style hover falloff dropped out and picked
up again every few pixels of travel. The rail's `gap` moves into the
ticks' own `padding-block`: same pitch, hit boxes now tile.
- The hover preview waited 300ms before opening — Astryx's HoverCard
default, meant for a pointer crossing a wide row on its way somewhere
else. A tick is 22px of rail that nothing is on the way to, and the wait
is the one part of this hover with no motion in it. Now 120ms.
- The highlight glided 280ms to wherever a click landed, so crossing
twenty prompts read as the bar flying off across the rail. A click now
owns the highlight until its scroll settles: no glide, and the scroll
no longer walks the highlight through every prompt it passes.
- The first click into a session did nothing until the reader scrolled by
hand. See below.
That last one is a collision between Astryx's auto-follow lock and the
progressive transcript mount, and neither side is wrong on its own.
`useChatStreamScroll` unlocks on a scroll up, detected by comparing
scrollTop between events — but it ignores any scroll event that arrives
with a changed scrollHeight or offsetHeight, because Chrome fires those
when content resizes and they are not the reader moving. A jump into an
unmounted turn mounts it and the fill that follows changes scrollHeight
for several frames, so the jump's own scroll is invisible to the lock: it
stays on, and `scrollIfLocked` pulls the transcript back to the bottom.
Only a wheel gesture broke it, which takes a separate path in Astryx.
`holdJumpDestination` re-aims at the target on each height change until
the fill stops. The last of those scrolls lands with a stable height,
which is the one the lock finally reads as a scroll up. Measured on the
30-prompt fixture: clicking the first tick went to scrollTop 7042 (the
bottom) and now goes to 24 and holds.
The fixture grows from 8 prompts to 30 because the progressive mount's
initial window is 10 — at 8 the head of the transcript is already mounted
and the jump-into-unmounted-turns path never runs at all.
Coverage note: the e2e case for the first click is an end-to-end check,
not a guard. Whether the lock wins depends on which frame the fill lands
on relative to a smooth scroll still in flight, and it goes green against
the unfixed renderer often enough to be worthless as one. The guard is
the `holdJumpDestination` unit test, which drives the frames itself.
* fix(ui): own a rail jump through the mount instead of racing it
Review of #2923 found the jump's ownership bound to a clock rather than to
the navigation, and the e2e case that was supposed to guard it asserting
almost nothing. Both hold.
Jump ownership:
- A second click during a jump only replaced the target; the first click's
700ms timer still governed, and could clear the second jump mid-flight.
Each click now carries its own sequence and starts its own hold.
- The fixed window is gone. A hold runs until the progressive mount reports
the transcript filled AND nothing has moved for a few frames, so a long
transcript is never released mid-fill, and it ends the moment the reader
touches the transcript (wheel, touch, pointer, key) rather than outliving
their interest in it.
Chasing the "just release auto-follow" direction the review preferred found
that ChatLayout publishes no such seam, so this adds one — `unlockAutoFollow`
on `ChatLayoutContextValue`, exposing the scroll hook's existing `unlock`
(patch hunk + patches/README entry). It is necessary and it is not
sufficient, which the earlier framing got wrong:
- Astryx re-locks on any `scrollend` that settles near the bottom, and a
session that opens at the bottom produces exactly that while the mount is
still catching up. Releasing once at the click is undone before the jump
goes anywhere — traced: released at the click, landed at 154ms, dragged
back to the bottom by 166ms. The release is now re-asserted for the life
of the hold.
- Auto-follow is not the only thing moving the transcript. The progressive
mount's own scroll compensation holds the reader's position across each
fill step, and mounting the turn a jump asked for IS a fill step, so it
lands after the jump and restores the position the jump just left. That
one no seam can fix; it is what the hold is for.
Jumps also scroll instantly now, whatever the app's scroll-motion policy
says. A jump is a teleport the reader asked for, and an animated one does
not survive this surface: traced on the 30-prompt fixture, the smooth scroll
was cancelled by the mount's compensation and by the follow spring and
stalled two pixels from where it started.
Coverage:
- The first-click e2e case named the wrong turn (`[data-turn-id]` is the
first MOUNTED turn, whose top is already negative at the opening scroll
position, so an upper-bound-only check passed without the jump doing
anything). It now names `turn-prompt-rail-1`, bounds it on both sides, and
asserts that tick's `aria-current`.
- `emulateMedia` could not put that case on the production scroll path:
`resolveScrollMotionBehavior` collapses motion for ANY fixture, keyed on
`data-maka-e2e-fixture` rather than on the media query. Fixtures can now
ask for a behavior back (`scrollMotion`, per launch — it costs seconds of
settling per window, so only the case that needs it pays), with unit
coverage for the precedence: a fixture request never outranks a stated
preference for less motion.
- `holdJumpDestination`'s unit tests grew the two cases its rewrite is
about: it must not settle while the transcript is still filling, and it
must hand the transcript back the moment the reader touches it.
Verified 5/5 on the smooth-scroll fixture, where the previous revision lost
1 in 4. `quote-selection.spec.ts` flakes on this machine (1 in 4) at
upstream/main as well, unchanged by this branch.
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
claiming a guard that does not exist — the worst state to leave an
assumption in, since the next person reads the citation and stops looking.
Restore the two assertions the comment is actually about, plus the ladder
order #2348 exists to protect, in ONE file. Split across two they each
stayed green against the half they could not see, which is how the
stylesheet half got deleted without anything noticing:
- the ladder is declared on the --space-* scale in strictly increasing
order (retuning a rung is a design call and stays green; list gaps
meeting block gaps fails)
- every rung declared is actually spent by a rule, so the ladder cannot
become decoration while literals do the spacing
- the runtime hooks the selectors need are really emitted by MarkdownBody
— data-maka-contract, data-density, astryx-markdown-heading, data-level,
astryx-list-item. These names have a single upstream owner in Astryx and
appear in Maka only inside selectors, so a rename kills every rule at
once with nothing failing
- compact markdown still has exactly one caller, which is the bet the
comment names: the table carries heading TYPOGRAPHY on a density key,
and Astryx's own RFC says density must not do that
Why these are not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions and CSS-structure suites that
asserted how a rule was written. Nothing here asserts an appearance or a
rule's shape: three of the four are relations that must survive any
retune, and the fourth renders the real component and checks the join
between Astryx's output and Maka's selectors. Deliberately left unpinned,
because they ARE how the table is written: the adjacent-sibling gap form,
the `hr` rung, the ListItem padding reset, and the two heading size tiers.
The stylesheet-reading half no longer revives the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read; it
reads packages/ui/src/styles.css directly, in the package that owns it.
Seven mutations, each verified to fail the assertion that describes it:
inverting the ladder; taking a rung off the --space-* grid; hardcoding a
rung's usage; hardcoding it while naming the rung in a comment (this is
why comments are stripped first); renaming the contract wrapper; cutting
density off before it reaches Astryx; and adding a second compact caller.
Also state in the comment what the contract does and does not hold, so
the next reader does not have to open the test to find out.
Generated-by: Claude Code
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
#2462 and its stylesheet-side sibling in #2425, so the comment has been
citing a guard that does not exist — the worst state to leave an assumption
in, since the next person reads the citation and stops looking.
Restore only what cannot be noticed without a test. Both failure modes here
are silent: no error, no failing check, just spacing that quietly stops
being what the table declares, because a CSS selector that matches nothing
never complains.
- The table selects entirely on DOM Astryx generates at runtime —
data-density, astryx-markdown-heading + data-level, astryx-list-item.
Those names have one upstream owner and appear in Maka only inside
selectors, so a rename kills every rule at once. Astryx is bumped
regularly (0.4.0 in #2983, 0.4.3 in flight, plus the Dependabot minor
group), so this is a recurring event, not a hypothetical.
- Astryx's ListItem carries CONTROL row padding that `density` cannot reach
from outside. That padding is what inverted the ladder in #2348 — list
items ~10px apart against 4px paragraphs — and one rule neutralizes it.
Lose the rule and the original defect returns, silently.
- Plus the assumption the comment actually names: compact markdown still
has exactly one caller. The table carries heading TYPOGRAPHY on a density
key, which Astryx's own RFC says density must not do, so the key is
honest only while `compact` and "transcript" are the same set.
Deliberately NOT pinned: the ladder's declared values and their order. A
first draft asserted them, and it was the wrong instinct — a reversed
ladder has to be typed on purpose into four adjacent lines under a comment
explaining the order, and it is visible the moment anyone looks at a
transcript. It also would not have caught the defect it claimed to guard:
#2348's inversion came from the ListItem padding above, which a test
reading only the declared variables cannot see. Same for the
adjacent-sibling gap form, the `hr` rung and the two heading size tiers —
those are how the table is written, not what it promises.
Why this is not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions, and CSS-structure suites that
asserted how a rule was written. Two of the three assertions here render
the real component and check the join between Astryx's output and Maka's
selectors; the third asserts an outcome the table promises, not a syntax.
The stylesheet-reading half does not revive the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read — it
reads packages/ui/src/styles.css directly, in the package that owns it.
Six mutations, each verified to fail the assertion that describes it:
deleting the ListItem padding reset; un-zeroing it; un-zeroing it while
naming the reset in a comment (this is why comments are stripped first);
renaming the contract wrapper; cutting density off before it reaches
Astryx; and adding a second compact caller.
Generated-by: Claude Code
Sign up for freeto 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.

2 participants

@jackwener@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' test: delete low-value and redundant suite files by jackwener · Pull Request #2462 · apache/maka · GitHub
Skip to content

test: delete low-value and redundant suite files - #2462

Merged
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests
Aug 8, 2026
Merged

test: delete low-value and redundant suite files#2462
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests

Conversation

@jackwener

@jackwenerjackwener commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

Delete 102 low-value or redundant source test files across Desktop, UI, Runtime, Headless, CLI, and Core, reducing the source test-file surface from 988 to 886 (-10.3%).

This branch is now based on the current main. The cleanup deliberately keeps tests that are the only guard for an active dependency patch or a security-sensitive boundary.

Removed

  • Desktop markup, copy, presentation, chrome, and overlapping geometry/workbar coverage
  • UI localization/copy panels, quiet chrome, empty states, and presentation pins
  • Runtime thin wire and duplicate provider helper coverage
  • Headless duplicate A/B, toolchain, and prompt-summary coverage
  • CLI layout/completion coverage and Core copy/locale helpers

Kept after review

  • Active dependency-patch guards:
    • AI SDK tool-call index tracking
    • Astryx List accessible-name forwarding
    • Astryx new-message indicator behavior
  • Security-sensitive boundaries:
    • split-chunk tool-output secret redaction
    • permission escalation confirmation and persistence
    • task-ledger prompt injection/redaction wiring
    • Responses API store: false
    • session-title internal-instruction stripping
  • Root authority, credentials, storage safety, primary runtime/session/tool suites, and fail-safe CI planning

Additional change

  • CLI process-contract tests ignore Node's ExperimentalWarning: SQLite stderr noise while continuing to reject other stderr output.

Validation

  • git diff --check origin/main...HEAD
  • npm run format:check
  • npm test — all workspace tests passed after synchronizing with current main

GitHub CI is running on the updated head.

jackwenerand others added 8 commits August 8, 2026 01:38
Remove 122 source test files (~12% of packages/apps test files) that were
markup/copy contracts, overlapping e2e chrome, duplicate UI presentation
pins, and low-signal headless AB/toolchain matrix cases. Keep safety,
authority, and primary product unit paths.
Also tolerate Node SQLite ExperimentalWarning in CLI run process contracts
so stderr-empty assertions are not flaky on modern Node.
Bring back e2e journeys, headless matrix/export, catalog contract, AiSdkFlow,
and subscription non-leak coverage that are sole or primary gates. Keep the
remaining ~11.6% file-level reduction of true low-value markup/chrome/matrix
duplicates.
Keep the sole PROVIDER_REGISTRY×models.dev sync sweep and the provider
catalog tab/OAuth/slug contract. These are not markup-copy pins and have no
thinner remaining suite.
…lue-tests
# Conflicts:
#	apps/desktop/e2e/new-messages-indicator.spec.ts
#	apps/desktop/e2e/prompt-rail.spec.ts
#	packages/ui/src/__tests__/conversation-localization.test.tsx
@Astro-Han
Astro-Han merged commit 2f5c75b into mainAug 8, 2026
21 of 23 checks passed
@Astro-Han
Astro-Han deleted the cleanup/delete-low-value-tests branch August 8, 2026 03:28
UncertaintyDeterminesYou4ndMe pushed a commit to UncertaintyDeterminesYou4ndMe/maka-agent that referenced this pull request Aug 8, 2026
Part of apache#2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
apache#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Astro-Han pushed a commit that referenced this pull request Aug 8, 2026
Part of #2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Co-authored-by: 陆逊 <211125649+UncertaintyDeterminesYou4ndMe@users.noreply.github.com>
ARE404 added a commit to ARE404/maka-agent that referenced this pull request Aug 13, 2026
apache#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— apache#2161 pinned it against a containing block as tall as the conversation,
apache#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in apache#2462, and the multi-prompt fixtures it ran on in apache#2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
Astro-Han pushed a commit that referenced this pull request Aug 13, 2026
…2923)
* fix(ui): give the prompt rail's tick bars a box again
#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— #2161 pinned it against a containing block as tall as the conversation,
#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in #2462, and the multi-prompt fixtures it ran on in #2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
* fix(ui): make the prompt rail's hover and jump behave
Four things the rail got wrong once it was visible again, found by using it:
- A 4px gap between ticks was a band where the pointer was over the rail
and over no tick, so the dock-style hover falloff dropped out and picked
up again every few pixels of travel. The rail's `gap` moves into the
ticks' own `padding-block`: same pitch, hit boxes now tile.
- The hover preview waited 300ms before opening — Astryx's HoverCard
default, meant for a pointer crossing a wide row on its way somewhere
else. A tick is 22px of rail that nothing is on the way to, and the wait
is the one part of this hover with no motion in it. Now 120ms.
- The highlight glided 280ms to wherever a click landed, so crossing
twenty prompts read as the bar flying off across the rail. A click now
owns the highlight until its scroll settles: no glide, and the scroll
no longer walks the highlight through every prompt it passes.
- The first click into a session did nothing until the reader scrolled by
hand. See below.
That last one is a collision between Astryx's auto-follow lock and the
progressive transcript mount, and neither side is wrong on its own.
`useChatStreamScroll` unlocks on a scroll up, detected by comparing
scrollTop between events — but it ignores any scroll event that arrives
with a changed scrollHeight or offsetHeight, because Chrome fires those
when content resizes and they are not the reader moving. A jump into an
unmounted turn mounts it and the fill that follows changes scrollHeight
for several frames, so the jump's own scroll is invisible to the lock: it
stays on, and `scrollIfLocked` pulls the transcript back to the bottom.
Only a wheel gesture broke it, which takes a separate path in Astryx.
`holdJumpDestination` re-aims at the target on each height change until
the fill stops. The last of those scrolls lands with a stable height,
which is the one the lock finally reads as a scroll up. Measured on the
30-prompt fixture: clicking the first tick went to scrollTop 7042 (the
bottom) and now goes to 24 and holds.
The fixture grows from 8 prompts to 30 because the progressive mount's
initial window is 10 — at 8 the head of the transcript is already mounted
and the jump-into-unmounted-turns path never runs at all.
Coverage note: the e2e case for the first click is an end-to-end check,
not a guard. Whether the lock wins depends on which frame the fill lands
on relative to a smooth scroll still in flight, and it goes green against
the unfixed renderer often enough to be worthless as one. The guard is
the `holdJumpDestination` unit test, which drives the frames itself.
* fix(ui): own a rail jump through the mount instead of racing it
Review of #2923 found the jump's ownership bound to a clock rather than to
the navigation, and the e2e case that was supposed to guard it asserting
almost nothing. Both hold.
Jump ownership:
- A second click during a jump only replaced the target; the first click's
700ms timer still governed, and could clear the second jump mid-flight.
Each click now carries its own sequence and starts its own hold.
- The fixed window is gone. A hold runs until the progressive mount reports
the transcript filled AND nothing has moved for a few frames, so a long
transcript is never released mid-fill, and it ends the moment the reader
touches the transcript (wheel, touch, pointer, key) rather than outliving
their interest in it.
Chasing the "just release auto-follow" direction the review preferred found
that ChatLayout publishes no such seam, so this adds one — `unlockAutoFollow`
on `ChatLayoutContextValue`, exposing the scroll hook's existing `unlock`
(patch hunk + patches/README entry). It is necessary and it is not
sufficient, which the earlier framing got wrong:
- Astryx re-locks on any `scrollend` that settles near the bottom, and a
session that opens at the bottom produces exactly that while the mount is
still catching up. Releasing once at the click is undone before the jump
goes anywhere — traced: released at the click, landed at 154ms, dragged
back to the bottom by 166ms. The release is now re-asserted for the life
of the hold.
- Auto-follow is not the only thing moving the transcript. The progressive
mount's own scroll compensation holds the reader's position across each
fill step, and mounting the turn a jump asked for IS a fill step, so it
lands after the jump and restores the position the jump just left. That
one no seam can fix; it is what the hold is for.
Jumps also scroll instantly now, whatever the app's scroll-motion policy
says. A jump is a teleport the reader asked for, and an animated one does
not survive this surface: traced on the 30-prompt fixture, the smooth scroll
was cancelled by the mount's compensation and by the follow spring and
stalled two pixels from where it started.
Coverage:
- The first-click e2e case named the wrong turn (`[data-turn-id]` is the
first MOUNTED turn, whose top is already negative at the opening scroll
position, so an upper-bound-only check passed without the jump doing
anything). It now names `turn-prompt-rail-1`, bounds it on both sides, and
asserts that tick's `aria-current`.
- `emulateMedia` could not put that case on the production scroll path:
`resolveScrollMotionBehavior` collapses motion for ANY fixture, keyed on
`data-maka-e2e-fixture` rather than on the media query. Fixtures can now
ask for a behavior back (`scrollMotion`, per launch — it costs seconds of
settling per window, so only the case that needs it pays), with unit
coverage for the precedence: a fixture request never outranks a stated
preference for less motion.
- `holdJumpDestination`'s unit tests grew the two cases its rewrite is
about: it must not settle while the transcript is still filling, and it
must hand the transcript back the moment the reader touches it.
Verified 5/5 on the smooth-scroll fixture, where the previous revision lost
1 in 4. `quote-selection.spec.ts` flakes on this machine (1 in 4) at
upstream/main as well, unchanged by this branch.
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
claiming a guard that does not exist — the worst state to leave an
assumption in, since the next person reads the citation and stops looking.
Restore the two assertions the comment is actually about, plus the ladder
order #2348 exists to protect, in ONE file. Split across two they each
stayed green against the half they could not see, which is how the
stylesheet half got deleted without anything noticing:
- the ladder is declared on the --space-* scale in strictly increasing
order (retuning a rung is a design call and stays green; list gaps
meeting block gaps fails)
- every rung declared is actually spent by a rule, so the ladder cannot
become decoration while literals do the spacing
- the runtime hooks the selectors need are really emitted by MarkdownBody
— data-maka-contract, data-density, astryx-markdown-heading, data-level,
astryx-list-item. These names have a single upstream owner in Astryx and
appear in Maka only inside selectors, so a rename kills every rule at
once with nothing failing
- compact markdown still has exactly one caller, which is the bet the
comment names: the table carries heading TYPOGRAPHY on a density key,
and Astryx's own RFC says density must not do that
Why these are not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions and CSS-structure suites that
asserted how a rule was written. Nothing here asserts an appearance or a
rule's shape: three of the four are relations that must survive any
retune, and the fourth renders the real component and checks the join
between Astryx's output and Maka's selectors. Deliberately left unpinned,
because they ARE how the table is written: the adjacent-sibling gap form,
the `hr` rung, the ListItem padding reset, and the two heading size tiers.
The stylesheet-reading half no longer revives the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read; it
reads packages/ui/src/styles.css directly, in the package that owns it.
Seven mutations, each verified to fail the assertion that describes it:
inverting the ladder; taking a rung off the --space-* grid; hardcoding a
rung's usage; hardcoding it while naming the rung in a comment (this is
why comments are stripped first); renaming the contract wrapper; cutting
density off before it reaches Astryx; and adding a second compact caller.
Also state in the comment what the contract does and does not hold, so
the next reader does not have to open the test to find out.
Generated-by: Claude Code
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
#2462 and its stylesheet-side sibling in #2425, so the comment has been
citing a guard that does not exist — the worst state to leave an assumption
in, since the next person reads the citation and stops looking.
Restore only what cannot be noticed without a test. Both failure modes here
are silent: no error, no failing check, just spacing that quietly stops
being what the table declares, because a CSS selector that matches nothing
never complains.
- The table selects entirely on DOM Astryx generates at runtime —
data-density, astryx-markdown-heading + data-level, astryx-list-item.
Those names have one upstream owner and appear in Maka only inside
selectors, so a rename kills every rule at once. Astryx is bumped
regularly (0.4.0 in #2983, 0.4.3 in flight, plus the Dependabot minor
group), so this is a recurring event, not a hypothetical.
- Astryx's ListItem carries CONTROL row padding that `density` cannot reach
from outside. That padding is what inverted the ladder in #2348 — list
items ~10px apart against 4px paragraphs — and one rule neutralizes it.
Lose the rule and the original defect returns, silently.
- Plus the assumption the comment actually names: compact markdown still
has exactly one caller. The table carries heading TYPOGRAPHY on a density
key, which Astryx's own RFC says density must not do, so the key is
honest only while `compact` and "transcript" are the same set.
Deliberately NOT pinned: the ladder's declared values and their order. A
first draft asserted them, and it was the wrong instinct — a reversed
ladder has to be typed on purpose into four adjacent lines under a comment
explaining the order, and it is visible the moment anyone looks at a
transcript. It also would not have caught the defect it claimed to guard:
#2348's inversion came from the ListItem padding above, which a test
reading only the declared variables cannot see. Same for the
adjacent-sibling gap form, the `hr` rung and the two heading size tiers —
those are how the table is written, not what it promises.
Why this is not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions, and CSS-structure suites that
asserted how a rule was written. Two of the three assertions here render
the real component and check the join between Astryx's output and Maka's
selectors; the third asserts an outcome the table promises, not a syntax.
The stylesheet-reading half does not revive the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read — it
reads packages/ui/src/styles.css directly, in the package that owns it.
Six mutations, each verified to fail the assertion that describes it:
deleting the ListItem padding reset; un-zeroing it; un-zeroing it while
naming the reset in a comment (this is why comments are stripped first);
renaming the contract wrapper; cutting density off before it reaches
Astryx; and adding a second compact caller.
Generated-by: Claude Code
Sign up for freeto 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.

2 participants

@jackwener@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); test: delete low-value and redundant suite files by jackwener · Pull Request #2462 · apache/maka · GitHub
Skip to content

test: delete low-value and redundant suite files - #2462

Merged
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests
Aug 8, 2026
Merged

test: delete low-value and redundant suite files#2462
Astro-Han merged 8 commits into
mainfrom
cleanup/delete-low-value-tests

Conversation

@jackwener

@jackwenerjackwener commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

Delete 102 low-value or redundant source test files across Desktop, UI, Runtime, Headless, CLI, and Core, reducing the source test-file surface from 988 to 886 (-10.3%).

This branch is now based on the current main. The cleanup deliberately keeps tests that are the only guard for an active dependency patch or a security-sensitive boundary.

Removed

  • Desktop markup, copy, presentation, chrome, and overlapping geometry/workbar coverage
  • UI localization/copy panels, quiet chrome, empty states, and presentation pins
  • Runtime thin wire and duplicate provider helper coverage
  • Headless duplicate A/B, toolchain, and prompt-summary coverage
  • CLI layout/completion coverage and Core copy/locale helpers

Kept after review

  • Active dependency-patch guards:
    • AI SDK tool-call index tracking
    • Astryx List accessible-name forwarding
    • Astryx new-message indicator behavior
  • Security-sensitive boundaries:
    • split-chunk tool-output secret redaction
    • permission escalation confirmation and persistence
    • task-ledger prompt injection/redaction wiring
    • Responses API store: false
    • session-title internal-instruction stripping
  • Root authority, credentials, storage safety, primary runtime/session/tool suites, and fail-safe CI planning

Additional change

  • CLI process-contract tests ignore Node's ExperimentalWarning: SQLite stderr noise while continuing to reject other stderr output.

Validation

  • git diff --check origin/main...HEAD
  • npm run format:check
  • npm test — all workspace tests passed after synchronizing with current main

GitHub CI is running on the updated head.

jackwenerand others added 8 commits August 8, 2026 01:38
Remove 122 source test files (~12% of packages/apps test files) that were
markup/copy contracts, overlapping e2e chrome, duplicate UI presentation
pins, and low-signal headless AB/toolchain matrix cases. Keep safety,
authority, and primary product unit paths.
Also tolerate Node SQLite ExperimentalWarning in CLI run process contracts
so stderr-empty assertions are not flaky on modern Node.
Bring back e2e journeys, headless matrix/export, catalog contract, AiSdkFlow,
and subscription non-leak coverage that are sole or primary gates. Keep the
remaining ~11.6% file-level reduction of true low-value markup/chrome/matrix
duplicates.
Keep the sole PROVIDER_REGISTRY×models.dev sync sweep and the provider
catalog tab/OAuth/slug contract. These are not markup-copy pins and have no
thinner remaining suite.
…lue-tests
# Conflicts:
#	apps/desktop/e2e/new-messages-indicator.spec.ts
#	apps/desktop/e2e/prompt-rail.spec.ts
#	packages/ui/src/__tests__/conversation-localization.test.tsx
@Astro-Han
Astro-Han merged commit 2f5c75b into mainAug 8, 2026
21 of 23 checks passed
@Astro-Han
Astro-Han deleted the cleanup/delete-low-value-tests branch August 8, 2026 03:28
UncertaintyDeterminesYou4ndMe pushed a commit to UncertaintyDeterminesYou4ndMe/maka-agent that referenced this pull request Aug 8, 2026
Part of apache#2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
apache#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Astro-Han pushed a commit that referenced this pull request Aug 8, 2026
Part of #2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.
- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
+ runMakaEvalCli with stdout/stderr captured at the process-stream
seam and env overrides applied and restored around the call. The
non-Headless-root test keeps the real bin route as the representative
wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
exported main(argv), mirroring the executable footer exactly (thrown
error -> stderr + exit 2). The no-argument rejection keeps the real
subprocess as representative coverage of that footer itself,
including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
process-wide stream writers; safe for the sequential node:test runs
these files use.
runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
#2462 deleted that suite on main.)
Timing (node --test, local, warm build):
cli.test.js 21.8s -> 13.1s (spawns 20 -> 2)
contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
Co-authored-by: 陆逊 <211125649+UncertaintyDeterminesYou4ndMe@users.noreply.github.com>
ARE404 added a commit to ARE404/maka-agent that referenced this pull request Aug 13, 2026
apache#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— apache#2161 pinned it against a containing block as tall as the conversation,
apache#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in apache#2462, and the multi-prompt fixtures it ran on in apache#2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
Astro-Han pushed a commit that referenced this pull request Aug 13, 2026
…2923)
* fix(ui): give the prompt rail's tick bars a box again
#2580 moved the rail's tick onto Astryx's Button. The bar the tick draws
was a direct child of the flex tick and got blockified; the Button wraps
its children in a label span, so the bar went back to normal flow as an
inline box. An inline box takes no width or height, so every bar computed
to 0x0 and the rail shipped invisible in 0.1.9 and 0.1.10 — present in the
DOM, painting nothing.
`display: block` on the bar restores it. Measured on the new fixture at
1280x800: the rail's box goes from 8px wide (its own padding, ticks
contributing nothing) back to the designed 22px.
This is the third time the rail has failed by rendering and not painting
— #2161 pinned it against a containing block as tall as the conversation,
#2338 parked it under macOS's overlay scrollbar — and the second time it
reached a release. The e2e coverage that would have caught all three was
deleted in #2462, and the multi-prompt fixtures it ran on in #2656, so
this adds back the smallest thing that closes the gap:
- `chat-prompt-rail`, a plain 8-prompt conversation. The rail hides itself
below three prompts, so the shipped single-prompt fixture cannot show it
at all.
- `prompt-rail.spec.ts` with one test per past failure: bars have a real
box, the rail stays inside the scrollport at both scroll extremes, and a
tick is what the pointer lands on. Three tests where the deleted suite
had nine.
Verified the first test fails on the unfixed renderer with "Expected: > 0,
Received: 0" and passes with the fix. Neither a static CSS read nor a
jsdom unit test can see any of this: jsdom has no layout engine.
* fix(ui): make the prompt rail's hover and jump behave
Four things the rail got wrong once it was visible again, found by using it:
- A 4px gap between ticks was a band where the pointer was over the rail
and over no tick, so the dock-style hover falloff dropped out and picked
up again every few pixels of travel. The rail's `gap` moves into the
ticks' own `padding-block`: same pitch, hit boxes now tile.
- The hover preview waited 300ms before opening — Astryx's HoverCard
default, meant for a pointer crossing a wide row on its way somewhere
else. A tick is 22px of rail that nothing is on the way to, and the wait
is the one part of this hover with no motion in it. Now 120ms.
- The highlight glided 280ms to wherever a click landed, so crossing
twenty prompts read as the bar flying off across the rail. A click now
owns the highlight until its scroll settles: no glide, and the scroll
no longer walks the highlight through every prompt it passes.
- The first click into a session did nothing until the reader scrolled by
hand. See below.
That last one is a collision between Astryx's auto-follow lock and the
progressive transcript mount, and neither side is wrong on its own.
`useChatStreamScroll` unlocks on a scroll up, detected by comparing
scrollTop between events — but it ignores any scroll event that arrives
with a changed scrollHeight or offsetHeight, because Chrome fires those
when content resizes and they are not the reader moving. A jump into an
unmounted turn mounts it and the fill that follows changes scrollHeight
for several frames, so the jump's own scroll is invisible to the lock: it
stays on, and `scrollIfLocked` pulls the transcript back to the bottom.
Only a wheel gesture broke it, which takes a separate path in Astryx.
`holdJumpDestination` re-aims at the target on each height change until
the fill stops. The last of those scrolls lands with a stable height,
which is the one the lock finally reads as a scroll up. Measured on the
30-prompt fixture: clicking the first tick went to scrollTop 7042 (the
bottom) and now goes to 24 and holds.
The fixture grows from 8 prompts to 30 because the progressive mount's
initial window is 10 — at 8 the head of the transcript is already mounted
and the jump-into-unmounted-turns path never runs at all.
Coverage note: the e2e case for the first click is an end-to-end check,
not a guard. Whether the lock wins depends on which frame the fill lands
on relative to a smooth scroll still in flight, and it goes green against
the unfixed renderer often enough to be worthless as one. The guard is
the `holdJumpDestination` unit test, which drives the frames itself.
* fix(ui): own a rail jump through the mount instead of racing it
Review of #2923 found the jump's ownership bound to a clock rather than to
the navigation, and the e2e case that was supposed to guard it asserting
almost nothing. Both hold.
Jump ownership:
- A second click during a jump only replaced the target; the first click's
700ms timer still governed, and could clear the second jump mid-flight.
Each click now carries its own sequence and starts its own hold.
- The fixed window is gone. A hold runs until the progressive mount reports
the transcript filled AND nothing has moved for a few frames, so a long
transcript is never released mid-fill, and it ends the moment the reader
touches the transcript (wheel, touch, pointer, key) rather than outliving
their interest in it.
Chasing the "just release auto-follow" direction the review preferred found
that ChatLayout publishes no such seam, so this adds one — `unlockAutoFollow`
on `ChatLayoutContextValue`, exposing the scroll hook's existing `unlock`
(patch hunk + patches/README entry). It is necessary and it is not
sufficient, which the earlier framing got wrong:
- Astryx re-locks on any `scrollend` that settles near the bottom, and a
session that opens at the bottom produces exactly that while the mount is
still catching up. Releasing once at the click is undone before the jump
goes anywhere — traced: released at the click, landed at 154ms, dragged
back to the bottom by 166ms. The release is now re-asserted for the life
of the hold.
- Auto-follow is not the only thing moving the transcript. The progressive
mount's own scroll compensation holds the reader's position across each
fill step, and mounting the turn a jump asked for IS a fill step, so it
lands after the jump and restores the position the jump just left. That
one no seam can fix; it is what the hold is for.
Jumps also scroll instantly now, whatever the app's scroll-motion policy
says. A jump is a teleport the reader asked for, and an animated one does
not survive this surface: traced on the 30-prompt fixture, the smooth scroll
was cancelled by the mount's compensation and by the follow spring and
stalled two pixels from where it started.
Coverage:
- The first-click e2e case named the wrong turn (`[data-turn-id]` is the
first MOUNTED turn, whose top is already negative at the opening scroll
position, so an upper-bound-only check passed without the jump doing
anything). It now names `turn-prompt-rail-1`, bounds it on both sides, and
asserts that tick's `aria-current`.
- `emulateMedia` could not put that case on the production scroll path:
`resolveScrollMotionBehavior` collapses motion for ANY fixture, keyed on
`data-maka-e2e-fixture` rather than on the media query. Fixtures can now
ask for a behavior back (`scrollMotion`, per launch — it costs seconds of
settling per window, so only the case that needs it pays), with unit
coverage for the precedence: a fixture request never outranks a stated
preference for less motion.
- `holdJumpDestination`'s unit tests grew the two cases its rewrite is
about: it must not settle while the transcript is still filling, and it
must hand the transcript back the moment the reader touches it.
Verified 5/5 on the smooth-scroll fixture, where the previous revision lost
1 in 4. `quote-selection.spec.ts` flakes on this machine (1 in 4) at
upstream/main as well, unchanged by this branch.
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
claiming a guard that does not exist — the worst state to leave an
assumption in, since the next person reads the citation and stops looking.
Restore the two assertions the comment is actually about, plus the ladder
order #2348 exists to protect, in ONE file. Split across two they each
stayed green against the half they could not see, which is how the
stylesheet half got deleted without anything noticing:
- the ladder is declared on the --space-* scale in strictly increasing
order (retuning a rung is a design call and stays green; list gaps
meeting block gaps fails)
- every rung declared is actually spent by a rule, so the ladder cannot
become decoration while literals do the spacing
- the runtime hooks the selectors need are really emitted by MarkdownBody
— data-maka-contract, data-density, astryx-markdown-heading, data-level,
astryx-list-item. These names have a single upstream owner in Astryx and
appear in Maka only inside selectors, so a rename kills every rule at
once with nothing failing
- compact markdown still has exactly one caller, which is the bet the
comment names: the table carries heading TYPOGRAPHY on a density key,
and Astryx's own RFC says density must not do that
Why these are not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions and CSS-structure suites that
asserted how a rule was written. Nothing here asserts an appearance or a
rule's shape: three of the four are relations that must survive any
retune, and the fourth renders the real component and checks the join
between Astryx's output and Maka's selectors. Deliberately left unpinned,
because they ARE how the table is written: the adjacent-sibling gap form,
the `hr` rung, the ListItem padding reset, and the two heading size tiers.
The stylesheet-reading half no longer revives the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read; it
reads packages/ui/src/styles.css directly, in the package that owns it.
Seven mutations, each verified to fail the assertion that describes it:
inverting the ladder; taking a rung off the --space-* grid; hardcoding a
rung's usage; hardcoding it while naming the rung in a comment (this is
why comments are stripped first); renaming the contract wrapper; cutting
density off before it reaches Astryx; and adding a second compact caller.
Also state in the comment what the contract does and does not hold, so
the next reader does not have to open the test to find out.
Generated-by: Claude Code
Astro-Han added a commit that referenced this pull request Aug 21, 2026
styles.css told readers the "compact == transcript" assumption was held by
`__tests__/markdown-rhythm-dom-contract.test.tsx`. That file was deleted in
#2462 and its stylesheet-side sibling in #2425, so the comment has been
citing a guard that does not exist — the worst state to leave an assumption
in, since the next person reads the citation and stops looking.
Restore only what cannot be noticed without a test. Both failure modes here
are silent: no error, no failing check, just spacing that quietly stops
being what the table declares, because a CSS selector that matches nothing
never complains.
- The table selects entirely on DOM Astryx generates at runtime —
data-density, astryx-markdown-heading + data-level, astryx-list-item.
Those names have one upstream owner and appear in Maka only inside
selectors, so a rename kills every rule at once. Astryx is bumped
regularly (0.4.0 in #2983, 0.4.3 in flight, plus the Dependabot minor
group), so this is a recurring event, not a hypothetical.
- Astryx's ListItem carries CONTROL row padding that `density` cannot reach
from outside. That padding is what inverted the ladder in #2348 — list
items ~10px apart against 4px paragraphs — and one rule neutralizes it.
Lose the rule and the original defect returns, silently.
- Plus the assumption the comment actually names: compact markdown still
has exactly one caller. The table carries heading TYPOGRAPHY on a density
key, which Astryx's own RFC says density must not do, so the key is
honest only while `compact` and "transcript" are the same set.
Deliberately NOT pinned: the ladder's declared values and their order. A
first draft asserted them, and it was the wrong instinct — a reversed
ladder has to be typed on purpose into four adjacent lines under a comment
explaining the order, and it is visible the moment anyone looks at a
transcript. It also would not have caught the defect it claimed to guard:
#2348's inversion came from the ListItem padding above, which a test
reading only the declared variables cannot see. Same for the
adjacent-sibling gap form, the `hr` rung and the two heading size tiers —
those are how the table is written, not what it promises.
Why this is not what #2425/#2462 retired. Those PRs removed markup/copy
pins, duplicate presentation assertions, and CSS-structure suites that
asserted how a rule was written. Two of the three assertions here render
the real component and check the join between Astryx's output and Maka's
selectors; the third asserts an outcome the table promises, not a syntax.
The stylesheet-reading half does not revive the deleted desktop
css-test-helpers module or the renderer-wide CSS aggregate it read — it
reads packages/ui/src/styles.css directly, in the package that owns it.
Six mutations, each verified to fail the assertion that describes it:
deleting the ListItem padding reset; un-zeroing it; un-zeroing it while
naming the reset in a comment (this is why comments are stripped first);
renaming the contract wrapper; cutting density off before it reaches
Astryx; and adding a second compact caller.
Generated-by: Claude Code
Sign up for freeto 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.

2 participants

@jackwener@Astro-Han