Document @keywords internal as the default for internal helpers

[d-morrison]

Summary

Adds a subsection to coding-practices/function-documentation.qmd
covering when to use @keywords internal versus @noRd for hiding
internal helpers.

The function-documentation page already covers `@inheritDotParams` and
the general "document every function" principle, but doesn't currently
distinguish the two tags. The new subsection:

• Notes that both hide the function from the package's public surface.
• Tabulates the differences (Rd generation, pkgdown index, accessibility
  via `?pkg:::fn`).
• Recommends `@keywords internal` as the default for any non-public helper —
  documentation still exists and is reachable, the function just doesn't
  pollute the pkgdown reference.
• Reserves `@noRd` for cases where docs genuinely don't help (one-line
  shims).

Test plan

• Render the lab manual (`quarto render`) and verify the new section
  appears under "Function Documentation" with the table rendering
  correctly.
• Spot-check the existing prose around `@inheritDotParams` still
  reads well with the new section appended.

[Copilot]

Pull request overview

Adds guidance to the lab manual on how to document non-exported R helpers, clarifying the tradeoffs between @keywords internal and @noRd within the “Function Documentation” section.

Changes:

• Added a new subsection recommending @keywords internal as the default for internal helpers.
• Introduced a comparison table explaining behavioral differences between @keywords internal and @noRd.
• Added an example roxygen block demonstrating @keywords internal usage.

[github-actions]

PR Preview Action v1.8.1-2-g6ad689f
🚀 View preview at https://UCD-SERG.github.io/lab-manual/pr-preview/pr-276/
Built to branch gh-pages at 2026-06-18 03:11 UTC. Preview will be ready when the GitHub Pages deployment is complete.

[d-morrison]

@claude address #276 (review)

[claude]

Claude finished @d-morrison's task in 1m 2s —— View job

---

Addressed copilot review feedback

• Read changed file for context
• Fix misleading wording about "hiding from public surface"
• Commit and push changes

The wording on line 142 was updated to make it clear that:

1. A function is kept internal by omitting @export (which keeps it out of NAMESPACE)
2. The two roxygen tags (@keywords internal and @noRd) only control how documentation is generated and indexed for that already-non-exported function

Change made (coding-practices/function-documentation.qmd):

-Two roxygen tags hide a function from the package's public surface,
-and they behave differently:
+A helper is kept internal by omitting `@export` (which keeps it out of `NAMESPACE`).
+Once the function is non-exported, two roxygen tags control how its documentation
+is generated and indexed, and they behave differently:

|

[d-morrison]

@claude review

[claude]

Claude finished @d-morrison's task in 1m 17s —— View job

---

Code Review

• Read CLAUDE.md for repo guidelines
• Read the changed file (coding-practices/function-documentation.qmd)
• Review git diff
• Post review findings

---

The new section is well-structured, fills a real gap in the lab manual, and the previously flagged misleading wording (about "hiding from the public surface") has already been corrected in a prior commit. The core guidance is accurate and practical.

A few observations:

Minor issue — imprecise ?internal_helper shorthand (function-documentation.qmd line 155)

The prose says:

future maintainers (or future-you) can ?internal_helper and see the documentation

Using bare ?internal_helper only works if the package is attached (via library(pkg)). For a non-attached package, it would silently return nothing. Since the table already uses ?pkg:::fn notation, the prose should match:

-so future maintainers (or future-you) can `?internal_helper` and see
-the documentation —
+so future maintainers (or future-you) can run `?pkg:::internal_helper` and see
+the documentation —

Fix this →

---

Minor style inconsistency — capitalization in example (function-documentation.qmd lines 169–171)

All other @param / @returns descriptions in the file use sentence-case with a capital first letter (e.g., @param data A data frame..., @returns A dataframe...). The new example uses lowercase:

#' @param row a one-row data frame.
#' @returns a numeric scalar.

Consider capitalising to match:

#' @param row A one-row data frame.
#' @returns A numeric scalar.

Fix this →

---

Suggestion — table could note the help(package=) index too

The table's "Appears in pkgdown reference index?" column is accurate, but @keywords internal also hides the function from the base-R help(package = "pkg") index (the main package help listing). If the audience is likely to check that listing, a note or extra column could be useful. This is optional — the current scope is fine for the lab manual audience.

---

Overall the guidance is correct, clear, and a welcome addition. The two items above are polish-level and could be addressed before merging or in a follow-up.

[d-morrison]

@claude address #276 (comment).

one possible correction: check if ?pkg::function_name (no :::) works for functions with keywords internal

[claude]

Claude finished @d-morrison's task in 1m 48s —— View job

---

Addressed review feedback on ?internal_helper notation

• Gather context
• Check ?pkg::fn vs ?pkg:::fn for @keywords internal functions
• Implement fix in function-documentation.qmd
• Commit and push

---

On ?pkg::fn vs ?pkg:::fn

?pkg::fn (double colon) does not work for @keywords internal functions. In R, pkg::fn only resolves exported symbols — calling it on a non-exported function raises:

Error: 'fn' is not an exported object from 'namespace:pkg'

Since @keywords internal functions intentionally omit @export, they're not in NAMESPACE, so pkg::fn fails before the help lookup even runs. pkg:::fn (triple colon) explicitly accesses the internal namespace and works correctly for non-exported functions.

---

Changes made

Two fixes applied to coding-practices/function-documentation.qmd:

1. Notation fix (line 154) — matches the ?pkg:::fn notation already used in the table above:

-so future maintainers (or future-you) can `?internal_helper` and see
+so future maintainers (or future-you) can run `?pkg:::internal_helper` and see

2. Capitalisation fix (lines 172–173) — sentence-case to match other examples in the file:

-#' @param row a one-row data frame.
-#' @returns a numeric scalar.
+#' @param row A one-row data frame.
+#' @returns A numeric scalar.

· Branch: add-keywords-internal-guidance

[d-morrison]

@claude review

[claude]

Claude finished @d-morrison's task in 1m 14s —— View job

---

Code Review

• Read CLAUDE.md for repo guidelines
• Read the changed file (coding-practices/function-documentation.qmd)
• Review git diff
• Post review findings

---

The new section is accurate, clearly written, and a genuine improvement to the lab manual. All issues from the previous review round have been correctly resolved (misleading wording, ?pkg:::fn notation, capitalisation). A few remaining items below:

---

Minor issue — ... in function body without ... in signature (function-documentation.qmd lines 176–178)

The example function uses ... as a body placeholder:

score_one_row <- function(row) {
  ...
}

Because the signature is function(row) (no ... formal), R would raise Error: '...' used in an incorrect context if this were actually evaluated. The rest of the file uses ### function code here as the placeholder convention (line 33). While readers are unlikely to run this snippet verbatim, a syntactically invalid placeholder could confuse newer R users.

Suggested fix:

score_one_row <- function(row) {
  ### function code here
}

Fix this →

---

Optional — note that help(package = "pkg") is also suppressed

Line 156–157 says the function "stays out of the pkgdown reference and the package's main help page." This is accurate — @keywords internal also suppresses the entry from the base-R help(package = "pkg") listing. The table already captures this implicitly via the "pkgdown reference index" column, so no change is strictly needed. Worth noting only if the audience is likely to use the base-R help index as a navigation tool.

---

Observation — opening paragraph overlaps with page-level guidance

Lines 137–140 re-state "Document every function you write, including internal helpers." The very first line of the page makes the same point. The repetition serves as useful motivation for the section, so this is a style choice rather than an error — just worth knowing if there's ever a page-level rewrite.

---

Overall the content is correct, the prose is tight, and the section fills a real gap. The ... placeholder is the only mechanical issue worth fixing before merge.
· Branch: add-keywords-internal-guidance

[d-morrison]

Claude Code CLI (local session) is working on this — paws off until I'm done.

[d-morrison]

Addressed the one mechanical finding from the latest review: replaced the ... function-body placeholder in the @keywords internal example with ### function code here (line ~177) to match the file's placeholder convention and avoid a syntactically invalid R expression.

The other items in the review were explicitly marked Optional/Observation (style choices the reviewer said need no change), and the inline 'misleading wording' note about hiding from the public surface is already resolved by the current text (lines 142–144, which now explain a helper is kept internal by omitting @export).

@claude review

[claude]

Claude finished @d-morrison's task in 1m 1s —— View job

---

Code Review

• Read CLAUDE.md for repo guidelines (no CLAUDE.md present in this repo)
• Read the changed file (coding-practices/function-documentation.qmd)
• Review git diff
• Post review findings

---

All issues from prior review rounds have been correctly resolved. Here's the current state:

Previous finding	Status
Misleading "hiding from public surface" wording	✅ Fixed (lines 142–144)
?internal_helper → ?pkg:::internal_helper notation	✅ Fixed (line 154)
Lowercase @param/@returns descriptions	✅ Fixed (lines 172–173)
... placeholder → ### function code here	✅ Fixed (line 177)

The ... fix in the trigger commit is confirmed on line 177:

score_one_row <- function(row) {
  ### function code here
}

---

No new issues found. The section is accurate:

• The table correctly reflects the behaviour of @noRd vs @keywords internal (Rd generation, pkgdown visibility, ?pkg:::fn accessibility).
• The prose correctly separates the concern of keeping a function internal (omitting @export) from the concern of controlling how its docs are generated.
• The example is syntactically valid R, uses the file's standard placeholder convention, and includes the correct @param/@returns capitalisation.

This section is ready to merge.

---

· Branch: add-keywords-internal-guidance

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[d-morrison]

Claude Code CLI (local session) is working on this — paws off until I'm done.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

[d-morrison]

@claude review

[claude]

Claude finished @d-morrison's task in 2m 40s —— View job

---

Code Review

• Read CLAUDE.md / copilot-instructions.md for repo guidelines
• Read the changed file (coding-practices/function-documentation.qmd)
• Review git diff
• Post review findings

---

Three outstanding findings from the June 3 and June 17 Copilot reviews; all prior issues (misleading wording, ?pkg:::fn notation, capitalisation, ... placeholder) are correctly resolved.

---

Issue — "out of NAMESPACE" is inaccurate (function-documentation.qmd line 142)

This is the June 3 Copilot finding, which landed after the June 3 claude sign-off and hasn't been addressed yet.

Line 142 reads:

A helper is kept internal by omitting `@export` (which keeps it out of `NAMESPACE`).

The phrase "keeps it out of NAMESPACE" is technically wrong in a way that contradicts the table on line 149. The function is in the package namespace — that's exactly why pkg:::fn works (confirmed in the table: "Yes" for @keywords internal). What @export controls is whether the function appears in the exports section of the NAMESPACE file.

Suggested fix:

-A helper is kept internal by omitting `@export` (which keeps it out of `NAMESPACE`).
+A helper is kept internal by omitting `@export`
+(which keeps it out of the `NAMESPACE` exports).

Fix this →

---

Issue — semantic line break missing on line 142 (function-documentation.qmd line 142)

This is the June 17 Copilot finding. The copilot-instructions.md style guide explicitly requires: "ALWAYS line-break at the ends of sentences and long phrases … Break after approximately 60–80 characters."

Line 142 is 84 characters and is a single complete sentence. The split above (separating the parenthetical onto its own line) resolves both this finding and the accuracy finding together.

Fix this →

---

Issue — table has no link to source docs (function-documentation.qmd around line 150)

This is the June 17 Copilot finding. The table at lines 146–149 makes specific factual claims about @noRd vs @keywords internal (Rd generation, pkgdown visibility, ::: accessibility) without any pointer to the upstream documentation that would let readers verify those claims.

The @inheritDotParams section already establishes the pattern for this — it ends with:

For more details on `@inheritDotParams`,
see the [roxygen2 documentation on inheriting parameters](https://roxygen2.r-lib.org/articles/rd.html#inheriting-documentation).

A similar closing line for this section would satisfy the Copilot finding and align with the file's existing style. Something like:

For more details on `@keywords internal` and `@noRd`,
see the [roxygen2 documentation on special keywords](https://roxygen2.r-lib.org/articles/rd-other.html#keywords)
and [pkgdown's reference index documentation](https://pkgdown.r-lib.org/articles/pkgdown.html#reference).

Fix this →

---

Summary

Finding	Source	Status
Misleading "hiding from public surface" wording	May 2026 Copilot	✅ Fixed
?internal_helper → ?pkg:::internal_helper	Jun 3 claude	✅ Fixed
Lowercase @param/@returns	Jun 3 claude	✅ Fixed
... placeholder → ### function code here	Jun 3 claude	✅ Fixed
"out of NAMESPACE" inaccuracy (line 142)	Jun 3 Copilot	❌ Open
Semantic line break on line 142	Jun 17 Copilot	❌ Open
No source links for table claims (line 150)	Jun 17 Copilot	❌ Open

The content is accurate and a clear improvement to the lab manual. The three open items above are all straightforward fixes before merge.
· Branch: add-keywords-internal-guidance