Add comprehensive guide for adding C++ modules to FIMS

[Copilot]

Adds and refines developer documentation for creating new C++ modules in FIMS, with the guide updated to match the current dev branch architecture.

Changes Made

Documentation

• vignettes/adding-new-module.Rmd: Comprehensive guide for adding new modules, revised to reflect the current dev branch workflow and architecture
• CONTRIBUTING.md: Expanded the “Adding a new module” section to incorporate the practical checklist content directly and added a dedicated Standard contributor checks section for routine contributor tasks
• README.md: Added developer-resource links to the module guide

Guide Improvements Based on Review Feedback

• Added short role summaries for the base class, implementation class, and module umbrella header
• Made the Step 1 “Key elements” formatting more consistent and explanatory
• Expanded the testing section to explain:
  • the purpose of C++ gtests versus R testthat tests
  • what sufficient module coverage should usually include
• Moved documentation expectations earlier in the guide so contributors see them before implementation is mostly complete
• Simplified the vignette by pointing developers to the relevant section in CONTRIBUTING.md for standard contributor tasks such as building Doxygen, running the Google test suite, running testthat, formatting, spell-checking, and package checks, rather than duplicating that guidance

Updated for Current dev Branch Architecture

The guide now reflects the current setup path and object lifecycle in dev, including:

• R-side setup workflow through:
  • R/create_default_configurations.R
  • R/create_default_parameters.R
  • R/initialize_modules.R
  • initialize_fims()
• Current Rcpp/TMB integration patterns, including:
  • add_to_fims_tmb_internal()
  • info->variable_map[...]
  • finalize()
• Current Rcpp exposure and module registration patterns in:
  • src/fims_modules.hpp
  • R/FIMS-package.R

Spell-check Support

• Added instantiation and lifecycle to inst/WORDLIST

Summary

This PR now provides both:

1. A detailed vignette describing how module development works in the current codebase
2. A practical checklist and standard contributor validation guidance embedded directly in CONTRIBUTING.md so the workflow is easier to maintain in one place

The updated documentation addresses the original request for:

• a written summary of the code structure and files that need to be modified
• links to the relevant scripts and code examples
• naming conventions and related documentation

It also incorporates reviewer suggestions and removes outdated or duplicated guidance so contributors are following the current dev-branch structure rather than an older FIMS architecture.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[feature:] create instructions on how to add a new C++ module to FIMS</issue_title>
<issue_description>### Describe the situation that led to the request and a solution

We have used template files before but we weren't able to keep them up-to-date which led to confusion. The architecture of FIMS is complex so it's hard for developers, especially those new to the team, to understand what needs to be changed when adding a new module to FIMS.

We would like to have instructions that include the following:

1. written summary description of the code structure and which files need to be modified (e.g. the file that creates the parameter object in R, the C++ code that loads the parameters, and any C++ code which uses that parameter)
2. links to the scripts that load parameters or examples in the code or merged pull requests where modules have been added
3. description of the naming conventions (or link to any existing documentation on naming conventions)

Alternative solutions

No response

Statistical validity

No response

Additional context

No response</issue_description>

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

• Fixes [feature:] create instructions on how to add a new C++ module to FIMS #1114

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[iantaylor-NOAA]

@kellijohnson-NOAA, @awilnoaa, and @grantdadams, take a look at the result of our breakout session issue assigned to copilot. On first glance, it's all pretty impressive, and the new vignette it created (vignettes/adding-new-module.Rmd) is complex enough that it's clear why we need something like this. Unfortunately I'm not qualified to judge the accuracy of the instructions.

One other thing that I'm curious about is what our process for updating the guide would look like. We could periodically ask copilot to update the guide, but I'm not sure if there would be a way to automatically trigger a check for whether the guide is out of date and/or propose updates. I'll ask copilot.

[iantaylor-NOAA]

@copilot, do you have suggestions for how to keep the documentation in this pull request up to date throughout future changes to the software architecture?

[Copilot]

@copilot, do you have suggestions for how to keep the documentation in this pull request up to date throughout future changes to the software architecture?

Here are several strategies to keep this documentation up to date:

1. Link-based Approach (Already Used)
The guide links to actual code files rather than copying code snippets. When files move or change significantly, the links will break during vignette building, alerting maintainers to update the guide.

2. CI Validation Check
Add a GitHub Actions workflow that:

• Builds the vignette during CI
• Verifies all file path references in the guide still exist
• Fails if referenced examples are missing

3. PR Template Addition
Add a checklist item to .github/pull_request_template.md:

- [ ] If this PR adds/modifies module structure, update `vignettes/adding-new-module.Rmd`

4. Automated Reminder
Create a GitHub Action that triggers on PRs modifying:

• inst/include/*/functors/*.hpp
• inst/include/interface/rcpp/rcpp_objects/rcpp_*.hpp
• src/fims_modules.hpp

The action posts a comment reminding reviewers to check if documentation needs updates.

5. Periodic Review
Schedule quarterly reviews (via GitHub issue templates or milestones) to verify documentation accuracy against current codebase.

Most Effective Combination: Use strategies 1, 3, and 4 together - the link-based approach catches breaking changes, PR template reminds developers, and automation flags architectural changes.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.99%. Comparing base (df16a64) to head (7ac4937).
⚠️ Report is 2 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1115      +/-   ##
==========================================
+ Coverage   81.07%   88.99%   +7.92%     
==========================================
  Files          53       80      +27     
  Lines        2087     8008    +5921     
  Branches      536      539       +3     
==========================================
+ Hits         1692     7127    +5435     
- Misses        303      857     +554     
+ Partials       92       24      -68     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[iantaylor-NOAA]

@copilot can you switch this PR to merge into dev instead of main?

[Copilot]

@copilot can you switch this PR to merge into dev instead of main?

I don't have the ability to change the base branch of a PR through git commands - this requires GitHub UI access or API permissions that I don't have.

To change the base branch from main to dev:

1. Go to the PR page on GitHub
2. Click "Edit" next to the base branch name at the top of the PR
3. Select dev from the dropdown
4. Confirm the change

Alternatively, a repository maintainer with appropriate permissions can make this change, or you can use the GitHub CLI:

gh pr edit <PR_NUMBER> --base dev

[kellijohnson-NOAA]

@copilot can you please update inst/WORDLIST to include instantiation and lifecycle

[Copilot]

@copilot can you please update inst/WORDLIST to include instantiation and lifecycle

Added both words to inst/WORDLIST in alphabetically correct positions (commit 5e14f66):

• "instantiation" added at line 700
• "lifecycle" added at line 267

[grantdadams]

Wow, that is super slick and helpful @iantaylor-NOAA for R&D work!

[kellijohnson-NOAA]

A lot of relevant information also now exists in two vignettes that will be brought in with #1089

[awilnoaa]

Hi @kellijohnson-NOAA , @iantaylor-NOAA , @grantdadams. I was reviewing the guide and as someone currently developing a new module, I found it really useful. I have a few suggestions to consider.

1. Step 1: C++ Implementation Section

• Add short role summaries for each class type. I think this would make the architecture clearer before diving into code details.
  • Include a one sentence purpose statement for each of the following, similar to the “Purpose” section in 1.3:
    • Base class
      • A brief description of what role the base class plays conceptually in FIMS, for example, defining the required interface and shared structure across implementations.
    • Implementation class
      • A short explanation that this is where the actual model specific math and parameter structure live.
    • Module (umbrella) header
      • A note that this file serves as the single include point that exposes all functors in that module category to the rest of the framework.

2. Make “Key elements” formatting consistent

• In Step 1, the “Key elements” sections are helpful, but I think they should mirror the “Key components” list in the Rcpp section. I suggest:
  • Using bullet list format
  • For each item, include a short description of why it matters

3. Step 5: Testing Section

• This section is well structured, but I thought it could use more guidance on why and what we are testing. As a new contributor, this would be helpful in understanding what “sufficient testing” looks like, in addition to where to put files.
  • Add a short description of the purpose of each test type:
    • C++ gtests: validating core math, logic, and behavior of the functors
    • R testthat tests: validating the Rcpp interface, object wiring, and user facing behavior
  • Include general guidelines for what a good module test should cover, for example:
    • Parameter setup and resizing
    • A known value test, as a sanity check against expected output
    • Edge or boundary behavior where relevant
    • Basic object lifecycle checks (IDs, object creation, etc.)

4. Step 6: Documentation Section

• This section is great, but it may be more useful earlier in the guide, perhaps near the Naming Conventions or early in Step 1. That way contributors can be aware of the documentation expectations before they start creating multiple files, rather than after implementation is mostly done.

5. Checklist Section

• I really like the complete checklist at the end. It is very helpful when working through a new module and making sure nothing is missed across layers. It makes the process feel more manageable.

If Copilot can make these updates, great, otherwise I am happy to update things. Let me know what you think. I am not sure if some of this material is already available elsewhere.

[kellijohnson-NOAA]

@awilnoaa do you have time to review this PR this week? I asked copilot to integrate your suggestions but I also asked it to remove the additional file of MODULE_CHECKLIST.md in favor of putting this information in CONTRIBUTING.md to reduce the locations where information is stored. I made some manual edits as well. Lastly, I asked copilot to update the instructions based on the current code in dev because we have changed a few things since the PR was made.

[awilnoaa]

@kellijohnson-NOAA will do

[awilnoaa]

I think this check might not be returning errors anymore. I tested it locally using

FIMSFrame(dplyr::filter(data_big, type != "weight_at_age"))

and it runs without producing an error. It looks like the final return(TRUE) / return(errors) section after the loop was removed and might need to be added back?

Example output:

r$> FIMSFrame(dplyr::filter(data_big, type != "weight_at_age"))

tbl_df of class 'FIMSFrame'
with the following 'types': age_comp, age_to_length_conversion, landings, length_comp, index
# A tibble: 6 × 8
# A tibble: 6 × 8
  type    name    age length timing value unit 
  <chr>   <chr> <int>  <dbl>  <int> <dbl> <chr>
1 age_co… flee…     1     NA      1 0.07  prop…
2 age_co… flee…     2     NA      1 0.1   prop…
3 age_co… flee…     3     NA      1 0.115 prop…
4 age_co… flee…     4     NA      1 0.15  prop…
5 age_co… flee…     5     NA      1 0.1   prop…
6 age_co… flee…     6     NA      1 0.05  prop…
# ℹ 1 more variable: uncertainty <dbl>

[awilnoaa]

Suggested change

	if (length(errors) == 0) {
	return(TRUE)
	} else {
	return(errors)
	}

[awilnoaa]

For this, it looks like if two separate PRs both target dev, one formatting run could cancel the other. Compared to the other workflow files, this seems like a new behavior, so I just wanted to double-check that it’s intentional.

[kellijohnson-NOAA]

@awilnoaa I need to rebase this PR. I am so sorry that I asked you to review this without realizing that it needed to be rebased.

[kellijohnson-NOAA]

I made your comments into separate issues, and have since rebased the PR so it is actually manageable. So sorry for that oversite.

[awilnoaa]

Everything else looks good to me

[awilnoaa]

I made your comments into separate issues, and have since rebased the PR so it is actually manageable. So sorry for that oversite.

No worries, lol I was wondering why there were so many files in this 😆