docs(skill): correct upstream-sync file mapping and add omit-vs-null pitfall#116
Merged
Merged
Conversation
Phase 10 self-review of the update-upstream skill after the v1.0.0-beta.12 sync surfaced documentation drift: - AGENTS.md Project Structure tree was missing `tool_set.clj` and the top-level `github.copilot-sdk` namespace (`copilot_sdk.clj`), which holds the curated public `event-types` / `session-events` sets. - references/PROJECT.md mapped the public `event-types` set to `client.clj` (it lives in `copilot_sdk.clj`) and referenced a non-existent `subscribe-events!`. Corrected, split curated-vs-generated event sets, and added the `toolSet.ts` -> `tool_set.clj` mapping. - SKILL.md gains pitfall #5: outbound optional wire fields must match upstream's omit-vs-null behavior per RPC (compute the value and gate on `(some? v)` when the request schema has no null variant, as with `session.model.switchTo`). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Documentation-only update to the update-upstream skill reference material, correcting drift between the skill docs and the current codebase (notably around event set locations and tool filtering), and capturing a recurring wire-level pitfall (omit vs null for optional RPC fields).
Changes:
- Add a new “omit-vs-null” pitfall for outbound optional RPC fields, with guidance to mirror upstream
undefined-omission semantics when schemas don’t allownull. - Fix the upstream↔Clojure mapping to point curated public event sets at
src/github/copilot_sdk.clj, distinguish curated vs generated event sources, and add thetoolSet.ts→tool_set.cljmapping. - Update the project-structure listing used by Copilot instructions to include
src/github/copilot_sdk.cljandtool_set.clj.
Show a summary per file
| File | Description |
|---|---|
.github/skills/update-upstream/SKILL.md |
Adds a concrete, generalized pitfall about omitting optional wire fields vs emitting JSON null, aligned with upstream behavior and schema unions. |
.github/skills/update-upstream/references/PROJECT.md |
Corrects sync mapping for curated event sets and tool filtering helpers; removes references to non-existent APIs and clarifies generated vs curated event sources. |
.github/copilot-instructions.md |
Brings the documented project structure in line with the actual src/ layout by listing the top-level public namespace file and tool_set.clj. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 10 self-review of the
update-upstreamskill, following the v1.0.0-beta.12 sync (#115). Documentation-only — corrects drift between the skill's reference material and the actual codebase.Changes
AGENTS.md(.github/copilot-instructions.md) — the Project Structure tree was missing two real files:tool_set.cljand the top-level public namespacesrc/github/copilot_sdk.clj(home of the curated publicevent-types/session-eventssets that sync work depends on).references/PROJECT.md— the upstream↔Clojure mapping pointed the publicevent-typesset atclient.clj(it lives incopilot_sdk.clj) and referenced a non-existentsubscribe-events!. Corrected, split the curated-vs-generated event sets into distinct rows, and added thetoolSet.ts→tool_set.cljmapping.SKILL.md— adds pitfall [docs] Populate documentation.instructions.md for AI agents #5: outbound optional wire fields must match upstream's omit-vs-null behavior per RPC. ThecontextTierlesson from Upstream sync: copilot-sdk v1.0.0-beta.12 (schema 1.0.57) #115, generalized — compute the wire value and gate on(some? v)when the request schema has no null variant (e.g.session.model.switchTo), rather thancontains?-gating and emitting JSONnull.Validation
bb validate-docs✓ (13 files, 0 warnings). No source or test changes.