Skip to content

Fix CI: update vdiffr snapshots, fix Rd docs, add Copilot setup#45

Draft
Copilot wants to merge 4 commits intomainfrom
copilot/replace-geom-line-with-geom-curve
Draft

Fix CI: update vdiffr snapshots, fix Rd docs, add Copilot setup#45
Copilot wants to merge 4 commits intomainfrom
copilot/replace-geom-line-with-geom-curve

Conversation

Copy link

Copilot AI commented Mar 5, 2026

  • Investigate CI failures (vdiffr snapshot tests failing in test-hgraph.R + documentation mismatch)
  • Generate updated SVG snapshots for hGraph tests using geom_curve output (with transition boxes)
  • Fix documentation mismatch in man/hGraph.Rd (add curvedflag and curvature parameters)
  • Add copilot-setup-steps.yml workflow (without Rust step)
  • Add .github/copilot-instructions.md with R CMD check instructions
  • Remove redundant curvedflag param; use curvature == 0 for straight lines (reviewer feedback)
  • Fix operator precedence bug: if/else inside ggplot + chain was silently dropping geom_rect and geom_text layers when curvedflag=TRUE (transition boxes were missing from all snapshots)
  • Regenerate snapshots with transition boxes now present
  • Update copilot-instructions.md: use R CMD check CLI, use roxygen2::roxygenize() for docs/NAMESPACE, merge documentation+NAMESPACE bullets
Original prompt

This section details on the original issue you should resolve

<issue_title>Replace geom_line() with geom_curve()</issue_title>
<issue_description>Idea from @keaven </issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…aph()

Co-authored-by: yihui <163582+yihui@users.noreply.github.com>
Copilot stopped work on behalf of yihui due to an error March 6, 2026 00:37
Co-authored-by: yihui <163582+yihui@users.noreply.github.com>
Copilot AI changed the title [WIP] Replace geom_line with geom_curve for better visualization Fix CI: update vdiffr snapshots, fix Rd docs, add Copilot setup Mar 6, 2026
Copilot finished work on behalf of yihui March 6, 2026 00:50
Copy link
Collaborator

@yihui yihui left a comment

Choose a reason for hiding this comment

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

why were transition boxes lost in the new svg snapshots?

Comment on lines +7 to +21
To run `R CMD check` locally:

```r
# Install check dependencies
install.packages("rcmdcheck")

# Run the check
rcmdcheck::rcmdcheck(args = c("--no-manual", "--as-cran"), error_on = "warning")
```

Or using devtools:

```r
devtools::check()
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

dont' use any packages; simply run command line R CMD check

Copy link
Author

Choose a reason for hiding this comment

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

Updated in ec59bf9 to use R CMD build . && R CMD check --no-manual --as-cran *.tar.gz.


### Common issues to watch for

- **Documentation mismatches**: If you add or rename function parameters, update the corresponding `man/*.Rd` file and the `@param` roxygen2 tags in the source `R/*.R` file.
Copy link
Collaborator

Choose a reason for hiding this comment

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

that's not the way to update documentation: run roxygen2::roxygenize() instead

Copy link
Author

Choose a reason for hiding this comment

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

Updated in ec59bf9 to use roxygen2::roxygenize().


Then commit the updated SVG files in `tests/testthat/_snaps/hgraph/`.

- **NAMESPACE**: If you add new imports or exports, update `NAMESPACE` using `devtools::document()`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

merge this item with the documentation item: in general, always run roxygen2 after making changes in source code to update documentation, namespace, etc.

Copy link
Author

Choose a reason for hiding this comment

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

Merged into a single "Documentation and NAMESPACE" bullet in ec59bf9.

R/hgraph.R Outdated
Comment on lines +173 to +174
curvedflag = TRUE,
curvature = 0.5,
Copy link
Collaborator

Choose a reason for hiding this comment

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

no need to add two args: curvature alone would be enough---curvature == 0 means segments

Copy link
Author

Choose a reason for hiding this comment

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

Removed curvedflag in ec59bf9. Now curvature = 0 draws straight segments (geom_segment), and any non-zero value draws curves (geom_curve).

…nerate snapshots, update copilot instructions

Co-authored-by: yihui <163582+yihui@users.noreply.github.com>
Copy link
Author

Copilot AI commented Mar 6, 2026

why were transition boxes lost in the new svg snapshots?

There was an operator precedence bug in R: if (curvedflag) { geom_curve(...) } else { geom_segment(...) } + geom_rect(...) + geom_text(...) is parsed as if (curvedflag) { geom_curve(...) } else { geom_segment(...) + geom_rect(...) + geom_text(...) } — so geom_rect and geom_text were only added to the else branch, and were silently dropped when curvedflag=TRUE. Fixed in ec59bf9 by assigning the conditional layer to a variable first (arrowLayer <- if (...) ...) and then adding it to the chain separately. The snapshots now include the transition boxes.

Copilot finished work on behalf of yihui March 6, 2026 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace geom_line() with geom_curve()

2 participants