Skip to content

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

Merged
climbfuji merged 2 commits into
feature/capgen-v1from
bugfix/feature_v1_haipeng_claude_review_1
Aug 27, 2026
Merged

[feature/capgen-v1] Fix three correctness findings from PR #762 review (@jimmielin/claude)#777
climbfuji merged 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

@climbfuji
climbfuji merged commit d249e94 into feature/capgen-v1 Aug 27, 2026
8 checks passed
@climbfuji
climbfuji deleted the bugfix/feature_v1_haipeng_claude_review_1 branch August 27, 2026 16:55
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.

4 participants