fix: build the review diff from Core's parsed analysis, not the document by Svilen-Stefanov · Pull Request #174 · CodeBoarding/CodeBoarding-action

Svilen-Stefanov · 2026-11-11T23:40:48Z

CodeBoarding/CodeBoarding#571 (merged) changes how Core writes analysis.json: format "v1". It drops methods_index, and a leaf's file_methods lists method ids instead of {file_path, methods} groups. This PR makes the review diff work with whichever format the pinned release writes, so the next re-pin onto a release with #571 doesn't break it.

Why it breaks today

load_analysis already parses both sides with the pinned release's parse_unified_analysis and rolls relations up with its project_relations_to_level. But the diff then read members, file hashes and method hashes from the raw document (file_methods groups, methods_index). On a v1 document that fails with 'str' object has no attribute 'get'. I reproduced this with main's script on Core main.

What changes

diff_to_mermaid.py reads only what Core parsed: each component's file_methods, the root's file index, and Core's projected relations at every level. The diff and the renderer are unchanged.
No reader or rollup of our own: both come from the installed release, as on main. The earlier version of this PR had its own analysis_reader.py and _project_relations. Both are gone.
Why one format is enough: the review analyses both commits with the pinned release. Where the review baseline starts from an analysis the release can't continue (another engine version, or a format it refuses), Core asks for a full run (requiresFullAnalysis) and analyze.sh runs it. So base and head are always in the format the pinned release reads.
core-compatibility CI: the job rendered the committed .codeboarding/analysis.json. After a pin moves, that file can still be in the previous release's format, which the new one refuses. It now runs InstalledCoreTests instead:
- tests/fixtures/analysis holds one analysis as Core 0.15.5 and Core main (9cf50fb9) wrote it;
- each release reads only its own: 0.15.5 fails on v1, and main raises AnalysisFormatError on legacy;
- the pinned release must read at least one, and every one it reads must give the same view;
- a future pin whose release reads neither fails the job until its own fixture is added.

Checked

On CodeBoarding's own analysis (two consecutive baseline syncs, 22 components, depth 1 and 2) and this repo's, the diagram and summary are byte-identical in three cases:

main's script on the legacy documents, with 0.15.5;
this PR on the legacy documents, with 0.15.5;
this PR on their v1 rewrites, with Core main.

Tests

python -m unittest discover -s tests without Core: 344 OK, 8 skipped
With codeboarding==0.15.5 installed: InstalledCoreTests and the provider drift test. InstalledCoreTests also passes with Core main installed.
black 25.9.0

Release order

Before the re-pin: this needs to merge before the Action pins a release that has #571.
Independent of feat: send the flow bundle as one xz document per side #171: that PR no longer depends on this one. Both add the same two fixture files with identical content, so they merge in either order.

🤖 Generated with Claude Code

chatgpt-codex-connector · 2026-11-11T23:40:57Z

Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-12T20:31:03.788228Z	`140f605`	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

codeboarding-review · 2026-11-11T23:41:04Z

CodeBoarding review

Status: 2 changed components

See the full change in CodeBoarding.

graph LR
    n_Repository_Analysis_and_Authentication_Tools["Repository Analysis and Authentication Tools"]
    n_Remote_Review_and_Briefing_Sync["Remote Review and Briefing Sync"]
    del_Action_Analysis_and_Authentication_Tools["Action Analysis and Authentication Tools"]
    n_Remote_Review_and_Briefing_Sync -- "authenticates requests via" --> del_Action_Analysis_and_Authentication_Tools
    classDef added fill:#1f883d,stroke:#0b5d23,color:#ffffff;
    classDef modified fill:#bf8700,stroke:#7d4e00,color:#ffffff;
    classDef deleted fill:#cf222e,stroke:#82071e,color:#ffffff,stroke-dasharray:5 3;
    class n_Repository_Analysis_and_Authentication_Tools added;
    class del_Action_Analysis_and_Authentication_Tools deleted;
    linkStyle 0 stroke:#82071e,stroke-width:2px,stroke-dasharray:5 3;

_{download artifacts · run 43300690316}

ivanmilevtues

it seems funcitonal but the versioning is not generalizable, can we fix that before merging.

ivanmilevtues · 2026-11-12T17:05:28Z

+    content_hash: str
+
+
+def is_v1(data: dict) -> bool:


this i don't like in particular.

Agreed, it's gone.

The version: it's named once, in CURRENT_FORMAT_VERSION.

The classification: read(document) classifies once, where the analysis is loaded:

no format_version is legacy;

the constant is current;

any other version raises. A higher vN says a newer CodeBoarding wrote the file and the Action needs updating; anything else is unsupported.

After loading: each format has one reader (_read_legacy, _read_current), and everything after read works on the result. No function checks a version any more.

Adding v2: one new reader plus moving the constant.

The v1 analysis was hand-written and broke the format Core writes: one-character ids, a raw-key member, all_edges on a nested relation, a top-level relation between parents and an edges table missing an edge. Its expected extra edge came only from that nested all_edges, and #174's reader, which now reads the single v1 form, fails on it. The pair is now one analysis written by Core's two writers, both after align_relation_edges at 3adb6969: legacy at main 53f0bb8e, v1 at 3adb6969. Each method sits in one leaf, top-level relations join leaves, and ids are Core's 12-character ones. The test still asserts both give the same side document, and which edges stay extra once the calls are left to the graph. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Svilen-Stefanov · 2026-11-12T18:58:28Z

@ivanmilevtues on the review ("the versioning is not generalizable"): Fixed before merge, along with the two minor review points.

The public API: the old functions are replaced by read(document) -> Analysis, with methods, members and edges. feat: send the flow bundle as one xz document per side #171 is restacked on it and adapted.
No version literals: nothing else in the Action compares against a version literal. Tests build current documents with the constant and are named by role.
Legacy groups without methods: they no longer create an empty file entry. That entry made a legacy base and a current head of the same analysis report a component as modified.
edges: nothing in fix: build the review diff from Core's parsed analysis, not the document #174 reads them. They're there for feat: send the flow bundle as one xz document per side #171's side document, and the description now says so.

The same change is in vscode#245, evals#38 and wrapper#111, and in Core#571's tests. graph-viewer#99 and webview#312 are handled separately.

load_analysis already parsed both sides with the pinned release's parse_unified_analysis and rolled relations up with its project_relations_to_level, but the diff then read members, file hashes and method hashes from the raw document: file_methods as {file_path, methods} groups and methods_index. Core #571's "v1" document has neither, so once the Action pins a release with it, the review diff fails with "'str' object has no attribute 'get'". The diff now reads only what Core parsed: each component's file_methods, the root's file index and Core's projected relations at every level. The review analyses both commits with the pinned release, so both sides are always in the format it reads, and a format change in Core needs no change here. The core-compatibility job rendered the committed .codeboarding/analysis.json, which after a pin moves can still be in the format of the release before, which the new one refuses. It now runs InstalledCoreTests: tests/fixtures/ analysis holds one analysis as Core 0.15.5 and Core main (9cf50fb9) wrote it, each release reads only its own, and every fixture the pinned one reads must give the same view. Checked on CodeBoarding's own analysis (two baseline syncs, 22 components, depth 1 and 2) and this repo's: output and summary are byte-identical to main's script on the legacy documents with 0.15.5, and to that again on their v1 rewrites with Core main, where main's script fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 140f60599c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-11-12T20:31:01Z

@@ -101,8 +126,7 @@ def load_analysis(path: Path) -> dict:
        root_analysis, sub_analyses = parse_unified_analysis(data)


Parse each stored analysis format independently

When the engine pin moves to the v1-writing release, a review can still use a legacy committed or stored baseline while producing a v1 head analysis. Both files are passed through the newly installed release's parse_unified_analysis, but that parser rejects the other release's format, so rendering exits before computing the diff. The compatibility test masks this by accepting success on any one fixture rather than requiring legacy/v1 mixtures; the action needs its own dual-format reader or another normalization path that can read both sides independently.

Useful? React with 👍 / 👎.

Svilen-Stefanov · 2026-11-12T20:38:46Z

/codeboarding

codeboarding-review · 2026-11-12T20:39:04Z

CodeBoarding review

Status: 2 changed components

See the full change in CodeBoarding.

graph LR
    n_Repository_Analysis_and_Authentication_Tools["Repository Analysis and Authentication Tools"]
    n_Remote_Review_and_Briefing_Sync["Remote Review and Briefing Sync"]
    del_Action_Analysis_and_Authentication_Tools["Action Analysis and Authentication Tools"]
    n_Remote_Review_and_Briefing_Sync -- "authenticates requests via" --> del_Action_Analysis_and_Authentication_Tools
    classDef added fill:#1f883d,stroke:#0b5d23,color:#ffffff;
    classDef modified fill:#bf8700,stroke:#7d4e00,color:#ffffff;
    classDef deleted fill:#cf222e,stroke:#82071e,color:#ffffff,stroke-dasharray:5 3;
    class n_Repository_Analysis_and_Authentication_Tools added;
    class del_Action_Analysis_and_Authentication_Tools deleted;
    linkStyle 0 stroke:#82071e,stroke-width:2px,stroke-dasharray:5 3;

_{download artifacts · run 43302371458}

It was squashed, so main holds its change but not its commits.

This was referenced Nov 11, 2026

Write each analysis.json fact once: an edges table and short method ids (format "v1") CodeBoarding/CodeBoarding#571

Merged

feat: send the flow bundle as one xz document per side #171

Merged

ivanmilevtues approved these changes Nov 12, 2026

View reviewed changes

Svilen-Stefanov force-pushed the feat/analysis-json-v1-reader branch from b026ec5 to 140f605 Compare November 12, 2026 20:28

Svilen-Stefanov changed the title ~~fix: read the review diff's analyses in Core's legacy and "v1" formats~~ fix: build the review diff from Core's parsed analysis, not the document Nov 12, 2026

chatgpt-codex-connector Bot reviewed Nov 12, 2026

View reviewed changes

Svilen-Stefanov merged commit 5334187 into main Nov 12, 2026
5 checks passed

Svilen-Stefanov added a commit that referenced this pull request Nov 12, 2026

Record that #174 landed on main

ee2d8c0

It was squashed, so main holds its change but not its commits.

Svilen-Stefanov added a commit that referenced this pull request Nov 12, 2026

Merge main into feat/flow-side-document, after #174 landed

23386ca

github-actions Bot mentioned this pull request Nov 12, 2026

chore(main): release 1.20.0 #175

Open

Navigation Menu

Repository navigation

fix: build the review diff from Core's parsed analysis, not the document - #174

fix: build the review diff from Core's parsed analysis, not the document#174
Svilen-Stefanov merged 1 commit into
mainfrom
feat/analysis-json-v1-reader

Svilen-Stefanov commented Nov 11, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Nov 11, 2026 •

edited

Loading

Uh oh!

codeboarding-review Bot commented Nov 11, 2026 •

edited

Loading

Uh oh!

ivanmilevtues left a comment

Uh oh!

ivanmilevtues Nov 12, 2026

Uh oh!

Svilen-Stefanov Nov 12, 2026

Uh oh!

Svilen-Stefanov commented Nov 12, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Nov 12, 2026

Uh oh!

Svilen-Stefanov commented Nov 12, 2026

Uh oh!

codeboarding-review Bot commented Nov 12, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -101,8 +126,7 @@ def load_analysis(path: Path) -> dict:
		root_analysis, sub_analyses = parse_unified_analysis(data)

Conversation

Svilen-Stefanov commented Nov 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why it breaks today

What changes

Checked

Tests

Release order

Uh oh!

chatgpt-codex-connector Bot commented Nov 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codeboarding-review Bot commented Nov 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodeBoarding review

Uh oh!

ivanmilevtues left a comment

Choose a reason for hiding this comment

Uh oh!

ivanmilevtues Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

Svilen-Stefanov Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

Svilen-Stefanov commented Nov 12, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

Svilen-Stefanov commented Nov 12, 2026

Uh oh!

codeboarding-review Bot commented Nov 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodeBoarding review

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Svilen-Stefanov commented Nov 11, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Nov 11, 2026 •

edited

Loading

codeboarding-review Bot commented Nov 11, 2026 •

edited

Loading

codeboarding-review Bot commented Nov 12, 2026 •

edited

Loading