Preserve assistant newline semantics after #584

[baron]

Summary

• replace PR #584's Character.isNewline loop with a hoisted CharacterSet.newlines scalar loop
• preserve the established Foundation newline-scalar contract, including counting CRLF as two separators
• add a dedicated five-test, 26-scenario contract matrix for newline membership, edge cases, exactness, and preview agreement

Why this follows #584

This is a corrective follow-up to PR #584, not a rejection of its performance finding or a git revert.

#584 correctly identified a hot loop, but changed its iteration unit from Unicode scalars to extended-grapheme Character values. Swift exposes CRLF as one newline Character, while the unchanged previewSummary still uses components(separatedBy: .newlines), where CR and LF are separate Foundation newline separators.

At the 10-line collapse boundary, merged #584 can therefore report:

• bounded decision: 10 lines, exact
• preview summary: 11 lines, collapse required

That one-sided semantic change can suppress the collapsed assistant-message presentation.

The smallest correction is:

let newlines = CharacterSet.newlines
for scalar in text.unicodeScalars where newlines.contains(scalar) {

This preserves the pre-#584 scalar/CRLF behavior and realigns the decision with the existing preview contract.

Confirmed performance mechanism

Release-optimized SIL shows why the original inline form was slow: the Foundation CharacterSet.newlines getter, temporary allocation, destruction, and deallocation remain inside the scalar loop. The compiler does not hoist them across the Foundation boundary. Binding the set once removes that repeated work and avoids extended-grapheme segmentation.

The expert's 1M ASCII result was approximately:

Original inline set	#584 Character.isNewline	Hoisted scalar set
335 ms	40 ms	30 ms

Two independent optimized reproductions confirmed the ordering:

Run	Original inline set	#584 Character.isNewline	Hoisted scalar set
1	200.7 ms	30.5 ms	21.5 ms
2	274.9 ms	44.1 ms	26.7 ms

The hoisted scalar form also won on combining and emoji/ZWJ no-newline inputs. Character.isNewline retained a sub-microsecond advantage on one LF-dense early-exit shape, so this PR does not claim universal dominance; it fixes the reported long-line case while preserving the established contract.

Focused coverage

AgentAssistantLineDerivationTests protects 26 scenarios across five methods:

• LF, CR, VT, FF, NEL, U+2028, and U+2029
• CRLF explicitly counted as two scalar separators, including the 10-line collapse boundary
• empty input and every trailing-separator form
• combining and emoji/ZWJ graphemes without newlines
• limits 0, 9, 10, and 11 with exact count / isExact expectations
• bounded collapse-decision agreement with previewSummary

The CRLF boundary test was demonstrated red on merged #584 (10/exact versus 11/collapse) before the production correction.

Validation

Passed:

• make dev-test FILTER=RepoPromptTests.AgentAssistantLineDerivationTests: 5 tests, 0 failures
• authoritative root XCTest list contains all five exact IDs
• ledger verification: 3,442 executable entries reconciled
• make dev-lint
• make dev-swift-build PRODUCT=RepoPrompt
• commit/push contribution preflights, guardrails, and staged/outgoing secret scans

Known limitations (hence draft):

• a prior 55-minute full-root run on the branch's Fix agent message bubble line counting performance #584-era base completed with 24 failures in CodeMap/worktree timing, authority, and tool-catalog golden tests outside the changed surface; the tool-catalog golden was subsequently updated on main by Fix history tool catalog golden #585
• isolated reruns reproduced one CodeMap authority failure on that base
• non-disruptive live smoke could not connect because the running CE app did not expose MCP; no visible app launch or relaunch was performed

Intentionally separate follow-ups

This PR does not broaden the corrective boundary:

1. Replace previewSummary's full components array with a reviewed scalar-contract single-pass derivation; exact remaining-line counts still require O(n) scanning even if retained allocation is bounded.
2. Make GitService.isHexObjectID ASCII-hex strict; Character.isNumber currently accepts Unicode digits.
3. Profile large-streaming Markdown rescans before redesigning them.
4. Do not introduce a generic repository-wide line abstraction; existing byte/scalar/Character/display contracts intentionally differ.

[baron]

Code-level approval on exact head 4bf721f3fe60f8cfd30285b40ab66785d8378c8f: using Unicode scalars with CharacterSet.newlines correctly aligns newline counting with the preview split semantics, including CRLF and Foundation newline scalars.

Please rebase, drop the obsolete edit to the test-suite ledger deleted on current main, rerun required CI, and then mark the PR ready. The sole existing approval targets an older commit, so the refreshed exact head will need non-author review. As the PR author, I cannot self-approve formally.

[morluto]

Audit disposition — product-contract decision required (2026-08-14)

Hoisting CharacterSet.newlines outside the scalar loop appears to preserve the existing Foundation-based preview behavior while improving the hot path. The unresolved question is whether that existing behavior is actually the desired user-facing line contract: counting CRLF as two separators is internally consistent with components(separatedBy:), but unusual for displayed text.

Please make one explicit product decision for CRLF and the full newline-scalar set, then implement both bounded counting and preview derivation against that same contract on current main. Rebase and rerun the focused/live coverage after that decision. I would not merge semantics accidentally just because both current functions agree.

[morluto]

Deep-review assessment — 2026-08-14

Disposition: hold for an explicit product-level newline contract, then rebase. Hoisting CharacterSet.newlines is a real performance improvement over looking up the set inside the scalar loop, and it restores agreement with the existing Foundation-based preview path. The unresolved question is semantic: counting CRLF as two line separators is internally consistent with that implementation but unusual for user-visible line counting, where CRLF normally represents one line break.

Please define one intentional line contract for bounded collapse decisions and preview generation, rather than preserving an accidental Foundation component behavior. Add product-facing CRLF fixtures and rebase onto current main. Once the semantic decision is explicit, the optimized scanner itself is straightforward.