Skip to content

Slim agent instructions and reject CJK ideographs in changed files - #71

Merged
jeffrey-bin merged 3 commits into
mainfrom
chore/agent-context-slim
Nov 12, 2026
Merged

jeffrey-bin merged 3 commits into
mainfrom
chore/agent-context-slim

Conversation

@jeffrey-bin

Copy link
Copy Markdown

What and why

Claude Code injects a subdirectory AGENTS.md whenever an agent reads a file under that directory, so large nested files cost context on every touch.

  • apps/desktop/AGENTS.md: the Source Package and Release sections move verbatim to a new Packaging Details section of docs/desktop-release.md. A pointer stays behind and names when to read it. The file drops from 27 KB to 17 KB. One self-referential link and the apps/desktop/AGENTS.md link in the release doc now point to the new section.
  • Root AGENTS.md:
    • "cheap gates", "fast boundary checks" and "fast checks" become one term, fast checks, defined once under make check.
    • The English-only rule, previously stated three times, is stated once with a NO / OK example.
    • The shared-configuration and cleanup rules state the outcome instead of the steps.
    • A duplicate pointer to the PR checks is removed.
  • Repository gate: changed files that contain CJK ideographs now fail, which enforces the English-only rule. Kana, Hangul and full-width punctuation stay allowed, so tests can still use unspaced scripts.

How it was verified

  • Focused tests: pnpm exec vitest run scripts/ci/check-repository-gate.test.ts, 7 passed. The new case covers both rejected ideographs and allowed kana.
  • make check-docs: all Markdown links resolve. Prettier and commitlint pass.
  • make check-scripts-fast has one failure, desktop-release.test.ts › "keeps the archive folder's address out of the log". It fails the same way on origin/main without this change.
  • Loading check: a fresh Claude Code session that reads apps/desktop/build.mjs follows the pointer and reaches the moved section. A session that works in apps/web does not.
  • Contracts: not touched.

Boundary

  • The moved text is unchanged apart from its heading level and the one link noted above.
  • Paths in the moved section are relative to apps/desktop/. A note at the top of the section lists the root-level scripts it mentions.
  • No change to the user message → provider_ready path.

🤖 Generated with Claude Code

@linger-review linger-review Bot 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.

Approve with nits (3 P3)

Scope: base main, merge base 1843eff, 3 commits (one a merge), 5 files reviewed. Loaded coderbook, ponytail-review and karpathy-guidelines. Ran in a separate session.

Findings

P3

  1. The CJK check passes silently when git grep errors (scripts/ci/check-repository-gate.sh:66)
    The check is if ... && git grep ...; then fail; fi. git grep returns 0 on a match, 1 on no match and 128 on an error. Because the if treats 128 like "no match", any error lets the gate pass. Examples: a git built without PCRE (-P unsupported), an over-long argument list, or a filename that git reads as pathspec magic. I confirmed locally that a bad pattern returns rc=128, and in this construct the gate would then continue. CI on ubuntu has PCRE, so CI is not at risk today. Local runs can still fail open. Fix: capture the status and fail on anything other than 0 or 1, e.g. git grep ... && rc=0 || rc=$?; ((rc == 1)) || { ((rc == 0)) && echo "...English-only..." >&2; exit 1; }.

  2. One file on main already contains ideographs (apps/voice-runtime/src/live-worker.test.ts:419)
    expect(afterCall("...", "...")) uses literal Chinese strings. This is the only tracked file that matches the new pattern. The gate scans whole changed files, not only changed lines. So the next PR that touches this test, even for an unrelated reason, fails the gate. Converting these two literals to \u escapes here would leave main clean when the rule lands.

  3. The path note in the moved section misses one root-level path (docs/desktop-release.md:146)
    The note says paths are relative to apps/desktop/ except scripts/prepare-native-*.mjs, scripts/source-native.mjs and scripts/installs/. The moved text also cites packages/local-installation/src/native-dependencies.ts, which exists only at the repository root. Add it to the exception list.

Verified as fine

  • The moved section matches the old apps/desktop/AGENTS.md "Source Package" and "Release" text line for line. Apart from heading levels and one trailing blank line, the only change is the link to docs/desktop-release.md, which became #how-the-workflow-runs. That anchor exists, and so does #packaging-details, which the desktop pointer and the release doc now link to.
  • The relative paths scripts/build-teamhub.mjs, scripts/build-native-runtime.mjs, packaging/... and src/main/distribution.test.ts all exist under apps/desktop/. scripts/prepare-native-*.mjs, scripts/source-native.mjs and scripts/installs/stack.sh exist at the root, which matches the note.
  • Root AGENTS.md: the rewording keeps every rule. "Fast checks" is defined once. The English-only scope (docs, code, comments, logs, strings, prompts, Skills, tests, fixtures, benchmark data) is preserved in a single place, and the removed sentence "Comments are English" is covered by it.
  • The regex (*UTF)[\x{3400}-\x{4DBF}\x{4E00}-\x{9FFF}\x{F900}-\x{FAFF}] under LC_ALL=C matches ideographs and does not match hiragana or full-width punctuation. I checked this in a temp repo. The published built-in Skill versions skipped by the loop are also excluded from the scan. The check runs before make check-scripts-fast check-docs, and the new test asserts both that order and the allowed-kana case. The fixture uses escaped code points, so the test file itself stays English-only.
  • No user message → provider_ready path, contracts, env vars or cross-app imports are touched.

Process notes

  • The branch contains a merge commit (01736d9 Merge remote-tracking branch 'origin/main'). main only takes rebase merges, so please rebase onto main instead of merging it in.
  • The branch is not behind main.

CI

repository-gate, check (desktop), check (web) and CodeQL were still pending at review time. The other checks were skipped by path filters. I could not run scripts/ci/check-repository-gate.test.ts locally because the worktree has no installed dependencies. Instead I checked the regex and the error exit code directly with git grep in a temp repo.

Automated review by Claude Code


Automated review of 01736d9 by Claude Code.

@linger-review linger-review Bot 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.

Approve (0 findings)

Scope: follow-up on 01736d9, reviewing head abd8340 against main (merge base 1843eff); 4 commits, 6 files in total, with the incremental diff 01736d9..abd8340 (1 commit, 3 files) reviewed in detail. Helper skills loaded: coderbook. ponytail-review and karpathy-guidelines were not loaded for this small follow-up; their checklists were applied from the review instructions. Ran in a separate session.

Previous findings

  • P3 scripts/ci/check-repository-gate.sh: the gate passed silently when git grep errored. Resolved. Lines 68-77 now capture the exit status and handle each case: 0 fails the gate with the English-only message, 1 passes, and any other status fails with "could not run (git grep exited N)". Under set -e, the || cjk_status=$? form is the right way to keep the status.
  • P3 apps/voice-runtime/src/live-worker.test.ts:419: a literal Chinese fixture was left on main. Resolved. It now uses \u escapes, and git grep finds no ideographs left in the file.
  • P3 docs/desktop-release.md:146: the relative-path note left out a root-level path. Resolved. packages/local-installation/ is now listed as a repository-root path.

Findings

None.

Verified as fine

  • The new gate block runs only when changed_files is non-empty. That list holds only existing, non-deleted files (--diff-filter=ACMRT plus [[ -f ]]), so pathspecs from deleted files can't cause spurious errors. A very long argument list now fails loudly instead of passing.
  • The escaped fixture asserts the same strings as before, so the test's behavior is unchanged.
  • The existing gate test rejects changed files with CJK ideographs and allows kana still covers both the match and no-match paths. There is no test for the error path (status other than 0 or 1). That path is trivial, and this is not worth blocking on.
  • The branch is up to date with origin/main (0 commits behind).

Process notes

  • 01736d9 Merge remote-tracking branch 'origin/main' is a merge commit. main takes rebase merges, which drop merge commits, so follow main by rebasing instead.

CI

All required checks that ran pass: repository-gate, check (desktop), check (web), changes, CodeQL. The other app checks were skipped because their paths are untouched. I did not run any tests locally.

Automated review by Claude Code


Automated review of abd8340 by Claude Code.

jeffrey and others added 3 commits November 12, 2026 23:16
The repository is English-only. The repository gate now fails when a changed
file contains CJK ideographs. Kana, Hangul, and full-width punctuation stay
allowed, so tests can still use unspaced scripts.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Claude Code injects apps/desktop/AGENTS.md whenever a desktop file is read,
so the Source Package and Release sections move verbatim to the Packaging
Details section of docs/desktop-release.md, with a pointer left behind.

The root AGENTS.md names one term, fast checks, for what it called cheap
gates and fast boundary checks; states the English-only rule once; and
describes the shared-configuration and cleanup rules as outcomes.

Co-Authored-By: Claude Code <noreply@anthropic.com>
The check treated every non-zero git grep status as "no match", so a git
without PCRE let the gate pass. Only status 1 now means clean. The one
existing test with literal ideographs uses escaped code points, and the
packaging note lists packages/local-installation/ as a root path.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@jeffrey-bin
jeffrey-bin force-pushed the chore/agent-context-slim branch from abd8340 to b2df175 Compare November 12, 2026 15:17
@jeffrey-bin

Copy link
Copy Markdown
Author

Addressed the review in b2df175: the English-only check now fails on any git grep status other than 0 or 1; live-worker.test.ts uses escaped code points; the packaging note lists packages/local-installation/ as a root path; and the branch is rebased onto main with no merge commit.

🤖 Addressed by Claude Code

@linger-review linger-review Bot 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.

Approve (0 findings)

Follow-up on abd8340. Scope: base main, merge base 1843eff, 3 commits, 6 files reviewed at b2df175. I loaded coderbook, ponytail-review and karpathy-guidelines as instructions (Skill tool not invoked; the review followed their named sections) and ran the review alone in a separate session.

Previous findings

The previous review had no findings.

What changed since abd8340

git diff abd8340..b2df175 is empty: the final tree is the same as at the previous review. git range-diff shows only a rebase onto the new main (1843eff). In the docs commit, the rebase brought the moved packaging text up to date with main. "When a voice key is saved, Native extracts…" became "On every install and start, whether or not a voice key is saved…", and the paragraph about failed tool preparation and TEAMHUB_VOICE_MEDIA=off was added. Because main itself changed this text, the move picks up the current wording and does not bring back the old wording.

Findings

None.

Verified as fine

  • Moved text: I diffed the lines removed from apps/desktop/AGENTS.md against the lines added to docs/desktop-release.md. Apart from line-wrapping artifacts, the only differences are the ones the PR body lists:
    • the new ## Packaging Details heading and its root-path note;
    • ## → ### heading levels;
    • the release-doc link rewritten to #how-the-workflow-runs.
  • Pointer: apps/desktop/AGENTS.md:142 now says "see Source Package and Release". It points to the new stub section at line 247, which links to ../../docs/desktop-release.md#packaging-details.
  • Gate script (scripts/ci/check-repository-gate.sh):
    • The CJK scan checks only changed, existing files outside published Skill versions, the same set that goes to Prettier.
    • Exit status 1 (no match) passes.
    • Status 0 (match) and any other status (for example, git without PCRE) fail the gate.
    • The ranges cover CJK Ext-A, the Unified block and Compatibility Ideographs. Kana and Hangul stay allowed, as the rule intends.
  • Test: check-repository-gate.test.ts covers rejecting 你好 (and checks that the later make step never runs) and accepting kana.
  • apps/voice-runtime/src/live-worker.test.ts: the literal Chinese strings became \u escapes. In JS these are the same string values, so the test does the same thing. The file now passes the new gate.

Process notes

No merge commits and no fixup commits. Merge state is CLEAN.

CI

All checks that ran passed: repository-gate, check (desktop), check (web), CodeQL and Analyze. The others were skipped by path filters. No check listed for this PR runs apps/voice-runtime tests, so the escaped-literal change is checked by reading the code only. Because the diff from the previous review is empty, I ran nothing locally.

Automated review by Claude Code


Automated review of b2df175 by Claude Code.

@jeffrey-bin
jeffrey-bin added this pull request to the merge queue Nov 12, 2026
Merged via the queue into main with commit 4839b91 Nov 12, 2026
14 checks passed
@jeffrey-bin
jeffrey-bin deleted the chore/agent-context-slim branch November 12, 2026 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant