Skip to content

sync-skill: size a module split by line count, and diagnose a type-only gate - #60

Merged
zpyoung merged 1 commit into
mainfrom
sync-lessons/v1.4.202
Sep 14, 2026
Merged

zpyoung merged 1 commit into
mainfrom
sync-lessons/v1.4.202

Conversation

@zpyoung

@zpyoung zpyoung commented Sep 14, 2026 •

Copy link
Copy Markdown
Owner
Files Added Deleted Net
Test 0 0 0 0
Prod 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​104 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​104

Two lessons from the v1.4.202 sync (PR #59). Both cost the run real time and both reproduce.

1. A module split does not have to measure in thousands

The existing lesson says a residual "measured in thousands means the tag replaced the file with a
barrel", and that "one or two lines" is ordinary -X ours hunk damage. v1.4.202 landed squarely
between the two and the run read the drift as hunk damage at first.

Upstream moved every RPC param schema into src/shared/rpc-contract/*-params.ts, leaving four
src/main/runtime/rpc/methods/*-schemas.ts files as re-export barrels:

src/main/runtime/rpc/methods/client-ui-schemas.ts:    recorded +10/-2,  measured +249/-5    (241 lines -> 8)
src/main/runtime/rpc/methods/session-tabs-schemas.ts: recorded +50/-1,  measured +278/-14   (229 lines -> 14)
src/main/runtime/rpc/methods/worktree-schemas.ts:     recorded +2/-1,   measured +216/-18   (215 lines -> 18)
src/main/runtime/rpc/methods/artifacts.ts:            recorded +6/-4,   measured +44/-7     (82 lines -> 45)

A fork feature that grew a seam file could plausibly show +249. The line count is what
separates the two, so the edit makes it a per-drifted-file sweep rather than something you reach
for once a number looks big.

The same sweep surfaces a second shape the lesson did not name: now > fork > tag, where the merge
took upstream's additions but not its deletions and the file ends up holding both implementations.
Two files did that here (NativeChatToolRun.tsx 348/462/467, TerminalContextMenu.tsx
355/383/473). It is repaired exactly like a split.

2. Type 'string' does not satisfy the constraint 'never'

v1.4.202 added a compile-time RPC params parity gate. A fork helper wrapping a method array —
withArtifactProtectionProjection<TMethod extends ProjectableMethod>(methods: readonly TMethod[])
where ProjectableMethod declares name: string — widened the literal method names at the
inference site, and the gate collapsed to one error naming no method, in the gate's own file:

src/main/runtime/rpc/rpc-params-type-parity.ts(44,47): error TS2344:
  Type 'string' does not satisfy the constraint 'never'.

const type parameters do not fix it and neither does readonly [...T]; the constraint is what
widens. Finding which of 50 method arrays was responsible took a generated probe file, which the
entry now carries verbatim — it answers in one typecheck.

The entry also records the ordinary half (fork-only methods need a two-line seam into
UncataloguedMethod; the generated catalog self-heals) and the consequence worth reporting rather
than burying: projectGroup.delete and repo.rm become uncatalogued once the project-ledger
feature extends them with a main-side schema.

3. A one-line correction folded in

Re-measuring residuals with git diff --numstat "$TAG" HEAD reads the last commit, while
--verify-residuals reads the worktree. During this run the two disagreed — a re-measure reported
zero changes and the verifier then failed on the file that had actually moved. Dropping the second
ref fixes it.


Scope is .claude/skills/sync-upstream/references/sync-lessons.md only. No rule was added to
SKILL.md; nothing here belongs in ## Hard safety rules or the fix policy's never-list.

Left open for review, as Step 15 requires — it blocks nothing. The sync is merged
(23cae70e86) and v1.4.203-rc.0.zy01 is dispatched.

Copilot AI lite review requested due to automatic review settings September 14, 2026 15:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The final review comments request three documentation corrections before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Documentation-only update to sync-upstream lessons covering module splits, RPC type-gate diagnostics, and residual measurement.

Changes:

  • Adds line-count guidance for detecting module splits and duplicate implementations.
  • Documents generated probing for widened RPC method names.
  • Corrects residual remeasurement against the worktree.
File summaries
File Summary
.claude/skills/sync-upstream/references/sync-lessons.md Adds sync troubleshooting guidance. Three nit findings remain (2, 3, and 2 votes) covering split confirmation, executable drifted-path input, and the artifacts.ts glob mismatch.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +43 to +46
`src/shared/rpc-contract/*-params.ts` and left four `src/main/runtime/rpc/methods/*-schemas.ts`
files as re-export barrels — 241 lines down to 8, 229 to 14, 215 to 18. The drifts were
`+249/-5`, `+278/-14`, `+216/-18`: hundreds, not thousands, and a fork feature that genuinely
grew a seam file could plausibly show the same number. The line count is what decides, so run it
"$(git show "${UPSTREAM_TARGET}:$p" 2>/dev/null | wc -l | tr -d ' ')" \
"$(git show "${MERGE_HEAD_PRE}:$p" 2>/dev/null | wc -l | tr -d ' ')" \
"$(wc -l < "$p" | tr -d ' ')"
done < <drifted-paths>
Comment on lines +58 to +62
`tag` far below `fork` is a split. And `now` **above** `fork` is its own finding: the merge took
upstream's additions but not its deletions, so the file now holds both implementations.
v1.4.202 did that to `NativeChatToolRun.tsx` (tag 348, fork 462, merged 467, residual `+119/-0`)
and `TerminalContextMenu.tsx` (355 / 383 / 473). Both are repaired the same way as a split:
`git checkout "$UPSTREAM_TARGET" -- <path>`, then replay the fork delta onto the tag.
@zpyoung
zpyoung merged commit 723e257 into main Sep 14, 2026
27 checks passed
zpyoung added a commit that referenced this pull request Sep 14, 2026
* Show pending asks across worktree, tab, and palette status surfaces

- Add selectors and coverage for pending ask attention state
- Keep resolved asks queued without showing active attention status

* docs(sync-skill): size a module split by line count, and diagnose a type-only gate (#60)

* perf(ask): index pending asks once per queue snapshot

Both selectors rescanned every pane queue on each store tick, once per mounted
tab and worktree card. Cache one tab-id index per pendingAsksByPaneKey
reference and unify the two disagreeing pane-key splits on the first colon.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants