Repository navigation
feat: close the Graphle parity code gaps - #52
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (1)ℹ️ 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. |
|
@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 |
There was a problem hiding this comment.
🛡️ Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 358cdf69c0
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
|
@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. |
9500bea to
40e81ba
Compare
The command line and the web app each had their own loop that read a source's pages into the store, and neither passed the relations a page carries (sub-issues, assignees, repository membership) to items.upsert, so the store's hierarchy queries never had anything to answer from. graph-sync is a core package both can use: it groups scopes by source, writes each page as it is read, relations included, holds a transition or relation back until the model has both of its ends, and reports progress with a checkpoint that records the scopes and the filter its cursor was issued under, so an interrupted sync resumes where it stopped. It is bundled into process-graph as the process-graph/sync subpath.
A source that fails while it is read now ends syncScopes with an InterruptedSyncError: the source's failure as its message and cause, the checkpoint that continues the failing source after the last page read from it, the scopes of the sources not reached, and whether any page was written. A host such as the command line turns that into its own resume hint without re-deriving it from progress. A store's failure is still thrown as the store threw it.
sync now runs through graph-sync, so the relations a source reports reach the store, and each page is written as it is read rather than in one write at the end. The revision a sync reports is therefore the revision of its last page's write. children <id> and ancestors <id> read the stored child-of relations through the store's hierarchy queries: an element's direct sub-tasks, and what it is a sub-task of, nearest first with the distance.
…ub-tasks The app's own sync loop is replaced by graph-sync, so a sync keeps the sub-issue, assignee and repository relations its pages carry, and the device's sync record stores graph-sync's checkpoint. The loop's tests moved with it. The details panel reads the selected element's parents and direct sub-tasks from the store's hierarchy queries, again on every revision, and lists each as a button that selects it.
sync takes --state (open or done) and --label, each repeatable, and --order (created, updated or comments) with --direction (asc or desc), and passes them to the source as the contract's typed filter. The source's cursor carries the filter it was issued under, so the resume command printed after a failure repeats the filter's options. A source that cannot filter a scope refuses the sync; GitHub's refusal for a Projects v2 board is reported with its own message and the source failure exit status.
A "What to read" control beside the scopes sets the sync's typed filter: issues in some run states only, issues carrying any of some labels only, and the order the source lists them in. The choices are part of the address (state, label, order and direction, named as the command line names them) and go into every sync. A resumed sync reads under the filter its checkpoint was issued with, since a source's cursor is valid only under that filter. While a Projects v2 board is in the scope the control says GitHub cannot filter it, and a filtered sync shows GitHub's refusal in the failure notice.
Projecting a model by a declared projection is the projection layer's work and has nothing of React in it, but it lived in graph-react, where nothing below the UI layer could use it. It now lives in graph-projection, so it reaches the root entry, and graph-react keeps exporting it beside useProjection, which memoises it.
The emitters lived in graph-cli, a Node-only target the web app cannot depend on, so the app had no way to write a view as Mermaid or DOT without copying them. They move, byte for byte, into a new core package, graph-export, with the visible-escape text helpers they rely on, and viewText becomes the one writer of a projected view's text: Mermaid, DOT, or the projection as JSON. The command line's show writes through viewText, and its library still exports the emitters. process-graph carries graph-export at its root entry. The escaping tests split along the same line: the helpers' own tests moved with them, and the command line keeps the tests of what it writes.
Beside the view, a button per format hands the view to the person to save, written by graph-export's viewText from the projection the view draws, so the file is the text the command line's show writes for the same view. The screen tests check exactly that, against show over the same store. Saving goes through a new download port, which the browser fills with a blob behind a link.
The web app's sync tests moved to graph-sync, which builds its own pages, so the web fixtures' page builder and first page, and one of graph-sync's own pages, are used only where they are defined.
The store contract's own source imports trilean, so knip sees the dependency used and reported the ignore as unneeded. Only the source contract, whose declarations name trilean without any source file importing it, still needs it.
The details panel offers "Load what this links to" for an element whose linked work lives in repositories outside the scope: the issues it depends on and holds up, its parent and its sub-tasks. It adds every such repository to the scope at once and syncs them through the same scope change and resumable sync as adding one repository does, saying which repositories it will add. Issues carry no board membership, so the expansion is by repository. The screen fixture and the fake GitHub server each gain a sub-issue in another organisation's repository, beside an issue nothing links to that only a sync of that repository reads.
viewText wrote the JSON format with JSON.stringify, which escapes the C0 controls but leaves C1 controls, bidirectional and other format characters, separators and private-use characters raw. The command line's stdout escaped them on the way out, but a file the web app saves did not. viewText now writes JSON through terminalJson, the escaping the command line's --json already used, so every consumer gets JSON that shows nothing hidden and parses back to the same values. show --format json is unchanged for text without such characters; for text with them it now writes exactly what show --format json --json writes.
graph-sync now ends a sync a source interrupted with an InterruptedSyncError whose cause is what the source threw. The app's failure classification looks through it, so a rate limit still shows its reset time and a refused token still offers to change it.
40e81ba to
5b21b3a
Compare
|
@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. |
Closes the code gaps the Graphle parity re-score found: sync relations, the typed sync filter, export from the web app, and per-element expansion. All contract changes are additive within
v1.Sync writes relations. A new core package,
graph-sync(process-graph/sync), is the one sync both the command line and the web app run. It writes each page as it is read, passes thechild-of,assigned-toandpart-ofrelations toitems.upsert, and holds a transition or relation back until both of its ends are in the model. The command line gainschildren <id>andancestors <id>over the store's hierarchy queries, and the web details panel lists an element's parent and sub-tasks as links that select them. Because the command line now writes page by page, the revisionsyncreports is that of its last page's write.Typed sync filter.
sync --state open|done --label <l> --order created|updated|comments --direction asc|desc, with the filter's options repeated in the resume command a failure prints. The web app has a "What to read" control whose choices go into the address (state,label,order,direction, documented in the web README) and into every sync; a resumed sync reads under the filter its checkpoint recorded. GitHub's refusal to filter a Projects v2 board comes through with its own message in both, and the web control warns about it while a board is in the scope.Browser export. The Mermaid and DOT emitters moved byte for byte from graph-cli into a new core package,
graph-export, exposed at theprocess-graphroot, withviewTextas the one writer of a view's text. To reach it below the UI layer,projectandProjectedViewmoved from graph-react to graph-projection (graph-react still exports them). The web app downloads the current view as Mermaid, DOT or JSON, and a screen test checks each download equals the command line'sshowoutput for the same view over the same store.Expand neighbours. "Load what this links to" in the details panel adds every repository outside the scope that the element's dependencies, dependants, parent and sub-tasks live in, in one step, through the existing scope change and resumable sync. Issues carry no board membership in the model, so expansion is by repository. Covered by a screen test and an e2e test against the fake GitHub server, which now has a cross-organisation sub-issue.
Also drops a stale knip ignore (
trileanfor the store contract), which knip had been hinting at onmain.