Repository navigation
refactor: define the bound activity once, in graph-schema - #68
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Decision recorded: the removal of boundActivitySchema and BoundActivity from the process-graph/source/github and process-graph/store subpaths is a breaking change to the published surface, so it releases as a major. Neither known consumer (Switchboard, pipewright) imports the names from a subpath, so the cost is nominal, and a re-export under the old name would keep two import paths for one symbol. The one definition is exported from the root entry with the rest of the model types. |
|
@codex security review |
1 similar comment
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex security review |
A sync page's activities and the store's upserted activities must be one shape, since graph-sync passes a page's items straight to items.upsert. Each contract now pins its activity type exactly, and graph-sync checks that both give the same JSON Schema, accept and reject the same values, and that a sync page's items pass boundItemsSchema unchanged.
The source and store contracts each built their own boundActivitySchema by unwrapping modelSchema's element union. graph-schema now defines it beside modelSchema, from the same object schema as the model's activity, declared as the named interface BoundActivitySchema so declaration files that embed it name it rather than writing it out. Both contracts parse activities with it, and the GitHub issues adapter takes the BoundActivity type from graph-schema. The wire shape of both contracts' v1 is unchanged, which the earlier pinned tests hold. Closes #31 BREAKING CHANGE: process-graph/source/github and process-graph/store no longer export boundActivitySchema or BoundActivity; import them from the process-graph root entry.
5d9fff3 to
c6a3241
Compare
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
boundActivitySchemaandBoundActivitynow live in graph-schema besidemodelSchema, built from the same object schema as the model's own activity, and both contracts parse activities with that one schema. It is declared as the named interfaceBoundActivitySchema, asModelSchemais, and the declaration-emit test now checks that a consumer embedding it names it rather than writing it out.The first commit pins the wire shape on the old code: each contract's activity type exactly, and in graph-sync that both contracts give the same JSON Schema, accept and reject the same values, and that a sync page's items pass
boundItemsSchemaunchanged. Those tests pass unchanged after the refactor, and each contract now also checks that its activity schema is graph-schema's object itself.The contracts no longer export the name, so
process-graph/source/githubandprocess-graph/storeloseboundActivitySchemaandBoundActivity; both are exported from theprocess-graphroot entry instead. That is a breaking change for anyone importing them from a subpath, so the refactor commit carries aBREAKING CHANGEfooter and merging releases a new major version ofprocess-graph. The GitHub issues adapter now depends on graph-schema for the type.Closes #31