Skip to content

[feature/capgen-v1] Fix three correctness findings from PR #762 review (@jimmielin/claude) - #777

Open
climbfuji wants to merge 2 commits into
feature/capgen-v1from
bugfix/feature_v1_haipeng_claude_review_1
Open

[feature/capgen-v1] Fix three correctness findings from PR #762 review (@jimmielin/claude)#777
climbfuji wants to merge 2 commits into
feature/capgen-v1from
bugfix/feature_v1_haipeng_claude_review_1

Conversation

@climbfuji

Copy link
Copy Markdown
Collaborator

Fix three correctness findings from PR #762 review (@jimmielin/claude):

  • suite_resolver: build the backward transform expression from the conflict-resolved temp name, not the raw _l candidate, so a scheme local colliding with the temp no longer silently transforms the wrong variable — including the optional+transform path (case 4), where the pointer already targets the renamed temp.
  • metadata_table: raise a clear error when a [ccpp-table-properties] block is missing name and/or type (or the key is mistyped), instead of silently dropping the table or failing later with a misleading "variable outside any section" error.
  • suite_resolver: reject the vertical-flip + allocatable-host combination outright rather than silently dropping the flip and passing vertically-reversed data.

User interface changes?: No

Testing:
test removed:
unit tests: added 7 unit tests (1557 → 1564).
system tests:
manual testing:

- suite_resolver: build the backward transform expression from the conflict-resolved temp name, not the raw <name>_l candidate, so a scheme local colliding with the temp no longer silently transforms the wrong variable — including the optional+transform path (case 4), where the pointer already targets the renamed temp.
- metadata_table: raise a clear error when a [ccpp-table-properties] block is missing name and/or type (or the key is mistyped), instead of silently dropping the table or failing later with a misleading "variable outside any section" error.
- suite_resolver: reject the vertical-flip + allocatable-host combination outright rather than silently dropping the flip and passing vertically-reversed data.

Adds 7 unit tests (1557 → 1564).
@climbfuji climbfuji self-assigned this Aug 12, 2026
@climbfuji
climbfuji requested review from a team as code owners August 12, 2026 22:48
@climbfuji climbfuji added the capgen bugs, requests, etc. that involve ccpp_capgen label Aug 12, 2026
@climbfuji climbfuji changed the title Fix three correctness findings from PR #762 review (@jimmielin/claude) [feature/capgen-v1] Fix three correctness findings from PR #762 review (@jimmielin/claude) Aug 12, 2026
@climbfuji

Copy link
Copy Markdown
Collaborator Author

@jimmielin Please review and request changes or approve when you get a chance. Thanks!

@jimmielin

Copy link
Copy Markdown
Contributor

My apologies for the delay, looks good to me, thanks!

@dustinswales dustinswales left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jimmielin Thank you for identifying these cases.
Thank you @climbfuji for addressing them.
LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

capgen bugs, requests, etc. that involve ccpp_capgen

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants