Improve API documentation discoverability#1167
Improve API documentation discoverability#1167sbryngelson merged 14 commits intoMFlowCode:masterfrom
Conversation
Add API nav link to landing page, cross-links in user guide sidebar,
auto-generated module listings for all three Doxygen targets, and
enable SOURCE_BROWSER for click-through to annotated source code.
The gen_api_landing.py script scans src/{target}/*.fpp at build time
to produce docs/{target}/readme.md with explicit @ref links that
handle Fortran case-insensitive namespaces correctly.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds two doc-generation Python steps (API landing pages and header fixes), wires them as CMake custom targets into the Doxygen build, enables DOXYGEN WARN_LOGFILE and SOURCE_BROWSER, updates site navigation and many Fortran source Doxygen headers, and makes multiple public subroutine signature changes across simulation and common modules. Changes
Sequence Diagram(s)sequenceDiagram
participant CMake
participant GenAPI as gen_api_landing.py
participant FixBriefs as fix_file_briefs.py
participant Doxygen
participant HTML as DocsOut
CMake->>GenAPI: run gen_api_landing (custom command)
GenAPI-->>CMake: gen-api-landing.stamp
CMake->>FixBriefs: run fix_file_briefs (custom command)
FixBriefs-->>CMake: fix-file-briefs.stamp
CMake->>Doxygen: invoke doxygen (deps satisfied)
Doxygen->>HTML: generate HTML (SOURCE_BROWSER enabled, WARN_LOGFILE -> log)
HTML-->>CMake: docs build complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Add Doxygen @file/@brief blocks to 13 modules that were missing them (m_body_forces, m_igr, m_muscl, m_sim_helpers, m_surface_tension, m_finite_differences, m_mpi_common, m_nvtx, m_delay_file_access, m_phase_change, m_check_ib_patches, m_check_patches, m_simplex_noise) and fix 3 files with malformed comment openers (m_chemistry used !!> instead of !>, m_data_output and m_helper_basic started with !! continuation markers instead of !> openers). All file list pages now show descriptions for every entry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use explicit @ref for mixed-case Fortran modules (m_bubbles_EE/EL/ EL_kernels) so Doxygen resolves the lowercase namespace. Correct wrong module names in @file briefs: m_boundary_conditions_common -> m_boundary_common, m_patches -> m_ib_patches / m_icpp_patches. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add docs/fix_file_briefs.py that reads the actual module/program name from each Fortran source file and ensures the @file @brief matches, using @ref for mixed-case identifiers. Runs before Doxygen via CMake so new files, renames, and case issues are caught automatically. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR enhances the discoverability and completeness of MFC's API documentation by adding navigation links, auto-generating landing pages, and ensuring all source files have proper Doxygen headers.
Changes:
- Added API navigation link to the main landing page and cross-links in the user guide sidebar for easy navigation to Pre-Process, Simulation, and Post-Process API documentation
- Implemented automated API landing page generation and @file/@brief header fixing via Python build scripts that run before Doxygen
- Fixed 19 source files: added missing @file headers, corrected malformed Doxygen comment openers, updated stale module names, and added @ref syntax for mixed-case Fortran modules to ensure proper hyperlinking
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/index.html | Added API navigation link to 7-column grid on main landing page |
| docs/documentation/readme.md | Added API cross-links in Development section for all three components |
| docs/simulation/readme.md | Auto-generated API landing page with module list and cross-links |
| docs/pre_process/readme.md | Auto-generated API landing page with module list and cross-links |
| docs/post_process/readme.md | Auto-generated API landing page with module list and cross-links |
| docs/gen_api_landing.py | Build script that scans source files and generates API landing pages |
| docs/fix_file_briefs.py | Build script that auto-fixes @file/@brief headers to match actual module/program declarations |
| CMakeLists.txt | Integrated both scripts as dependencies of Doxygen targets |
| docs/Doxyfile.in | Enabled SOURCE_BROWSER to allow clickthrough to annotated source code |
| src/simulation/*.fpp (6 files) | Added missing @file headers or fixed @brief to use @ref for mixed-case modules |
| src/pre_process/*.fpp (3 files) | Added missing @file headers or corrected stale module names |
| src/common/*.fpp (7 files) | Added missing @file headers or fixed malformed comment openers |
| .lychee.toml | Excluded strawberryperl.com from link checking (frequently times out) |
docs/gen_api_landing.py
Outdated
| """Return sorted list of module names (m_*) from .fpp files.""" | ||
| return sorted( | ||
| f.stem for f in directory.glob("m_*.fpp") | ||
| ) |
There was a problem hiding this comment.
The get_modules function only scans for .fpp files but misses .f90 files that contain modules. This excludes several modules from the generated API landing pages, including m_nvtx, m_compile_specific, m_delay_file_access from common/, m_grid from pre_process/, and m_data_input from post_process/. The function should use glob patterns for both *.fpp and *.f90 files, similar to how fix_file_briefs.py handles both file extensions on line 68.
| """Return sorted list of module names (m_*) from .fpp files.""" | |
| return sorted( | |
| f.stem for f in directory.glob("m_*.fpp") | |
| ) | |
| """Return sorted list of module names (m_*) from .fpp and .f90 files.""" | |
| modules: set[str] = set() | |
| for pattern in ("m_*.fpp", "m_*.f90"): | |
| for f in directory.glob(pattern): | |
| modules.add(f.stem) | |
| return sorted(modules) |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 32 files
Confidence score: 4/5
- Incremental builds can leave generated headers and API landing pages stale because custom commands in
CMakeLists.txtdepend only on the scripts, so edits to .fpp sources won’t retrigger those steps - Overall risk is low since this mainly affects freshness of generated artifacts rather than core runtime behavior
- Pay close attention to
CMakeLists.txt- ensure custom commands depend on the relevant .fpp sources so rebuilds trigger correctly.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="CMakeLists.txt">
<violation number="1" location="CMakeLists.txt:813">
P2: The API landing page generator won’t rerun when source .fpp files change because the custom command only depends on the script. That leaves docs/{target}/readme.md stale on incremental builds. Add the relevant source lists to the DEPENDS so changes to modules retrigger generation.</violation>
<violation number="2" location="CMakeLists.txt:829">
P2: The @file/@brief fixer won’t rerun when .fpp sources change because the custom command only depends on the script. That makes header fixes stale on incremental builds. Add source file dependencies so edits retrigger the tool.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pre_process/m_check_ib_patches.fpp (1)
302-303:⚠️ Potential issue | 🟡 MinorMissed malformed Doxygen opener on
s_check_inactive_ib_patch_geometry.Line 302 uses
!!>which Doxygen parses as a plain!!continuation marker followed by a literal>— not a block opener. The PR explicitly fixes the same class of defect elsewhere (m_chemistry.fpp,m_helper_basic.fpp) but this instance was missed, leavings_check_inactive_ib_patch_geometrywithout a rendered documentation entry.📝 Proposed fix
-!!> This subroutine verifies that the geometric parameters of +!> This subroutine verifies that the geometric parameters of🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pre_process/m_check_ib_patches.fpp` around lines 302 - 303, The Doxygen opener for s_check_inactive_ib_patch_geometry is malformed: replace the leading "!!>" token with the correct Doxygen block opener used elsewhere (use "!!<" as the opener) so the subroutine docblock is recognized—update the comment header for s_check_inactive_ib_patch_geometry to use "!!<" and match the style used in m_chemistry.fpp/m_helper_basic.fpp (preserve the rest of the comment text and spacing).src/simulation/m_surface_tension.fpp (1)
39-39:⚠️ Potential issue | 🟡 MinorPre-existing malformed Doxygen group-close marker:
@)should be@}.Line 39 uses
!> @)to close the@{group opened on line 37, but@)is not a valid Doxygen command. The correct closing tag is@}. The PR's goal explicitly includes fixing malformed Doxygen comment openers — this one appears to have been missed.🐛 Proposed fix
- !> @) + !> @}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/simulation/m_surface_tension.fpp` at line 39, Replace the malformed Doxygen group-close marker "!> @)" with the correct "!> @}" in the Fortran module comments so the "@{" group opened earlier is properly closed; locate the comment line containing the exact token "!> @)" in m_surface_tension.fpp and update it to "!> @}" (no other code changes required).
🧹 Nitpick comments (4)
docs/index.html (2)
175-175:sm:grid-cols-3produces an orphaned row with 7 items.At the
smbreakpoint, 7 items fill 2 rows of 3 plus 1 lone item. Previously 6 items divided evenly. Considersm:grid-cols-4(gives 4+3) orjustify-items-centeron the grid wrapper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/index.html` at line 175, The grid at the div with class "justify-center grid grid-cols-2 sm:grid-cols-3 lg:grid-cols-7 gap-x-4 md:gap-x-8 text-md md:text-xl text-center text-white font-medium" creates an orphaned single item at the sm breakpoint; update the class to either replace sm:grid-cols-3 with sm:grid-cols-4 (so items lay out 4+3) or keep sm:grid-cols-3 and add justify-items-center to the class list to center the lone item—apply the change to that div's class attribute.
195-198: API nav points only to the Simulation target; Pre-Process and Post-Process require a second click.Users visiting the landing page expecting e.g. the Pre-Process API must navigate to the Simulation landing, then follow the "See Also" cross-link. An optional improvement would be a dropdown or a dedicated API page that lists all three targets, but the current cross-link approach is acceptable for a first iteration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/index.html` around lines 195 - 198, The "API" nav anchor (the <a> element with href="simulation/index.html" and inner <span>API</span>) currently points only to the Simulation landing; update navigation so the "API" link goes to a dedicated API index that lists Pre-Process, Simulation, and Post-Process (create a new api/index.html that contains links to each target), and change the anchor's href from "simulation/index.html" to "api/index.html" (alternatively implement a dropdown menu from that anchor that exposes the three targets). Ensure the anchor text remains "API" so users land directly on a page listing all three API targets.docs/gen_api_landing.py (1)
71-71: Fragile section-heading derivation viasplit()[-1].
info['title'].split()[-1]picks the last word of the title. For the three current targets this works (Pre-Process,Simulation,Post-Process), but it breaks silently if a title is ever restructured (e.g.,"MFC Pre Process Utility"would yield"Utility"). Consider using an explicit"section_title"key in theTARGETSdict.♻️ Proposed fix
TARGETS = { "pre_process": { "title": "MFC Pre-Process", + "section": "Pre-Process", ... }, "simulation": { "title": "MFC Simulation", + "section": "Simulation", ... }, "post_process": { "title": "MFC Post-Process", + "section": "Post-Process", ... }, } # then: - lines.append(f"### {info['title'].split()[-1]}") + lines.append(f"### {info['section']}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/gen_api_landing.py` at line 71, Replace the fragile title-splitting logic that builds section headings from info['title'].split()[-1] with code that first looks for an explicit 'section_title' key on the target metadata (i.e., in the TARGETS dict / the info dict used here) and uses that if present; if no 'section_title' is provided, fall back to the existing split()[-1] behavior for backward compatibility. Update the place that appends the heading (the lines.append call in docs/gen_api_landing.py) to use info.get('section_title', info['title'].split()[-1]) so maintainers can set explicit section names without breaking existing titles.CMakeLists.txt (1)
811-839: Stamp commands don't depend on.fppsource files — new modules won't trigger auto-regeneration.Both
gen-api-landing.stampandfix-file-briefs.stamplist only their Python scripts inDEPENDS. Adding or renaming an.fppmodule leaves the stamp newer than any tracked dependency, so CMake skips regeneration silently. The landing pages and file briefs stay stale until the stamp is manually deleted.Add the globbed
.fpplists as additional DEPENDS so any source-file change invalidates the stamp:♻️ Proposed fix
+ file(GLOB_RECURSE api_fpp_srcs CONFIGURE_DEPENDS + "${CMAKE_CURRENT_SOURCE_DIR}/src/pre_process/*.fpp" + "${CMAKE_CURRENT_SOURCE_DIR}/src/simulation/*.fpp" + "${CMAKE_CURRENT_SOURCE_DIR}/src/post_process/*.fpp" + "${CMAKE_CURRENT_SOURCE_DIR}/src/common/*.fpp") + add_custom_command( OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/gen-api-landing.stamp" DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/docs/gen_api_landing.py" + ${api_fpp_srcs} COMMAND "${Python3_EXECUTABLE}" ...Apply the same
${api_fpp_srcs}addition to thefix-file-briefscommand.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CMakeLists.txt` around lines 811 - 839, The stamp targets gen-api-landing.stamp and fix-file-briefs.stamp only depend on their Python scripts so adding/renaming .fpp modules won't retrigger them; update the add_custom_command DEPENDS for the gen_api_landing and fix_file_briefs commands to include the globbed Fortran source list (e.g. ${api_fpp_srcs} or the appropriate .fpp glob variable you use in this CMakeLists) so any .fpp change invalidates the stamp, and apply the identical change to the fix-file-briefs command to ensure both stamps regenerate when .fpp sources change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/fix_file_briefs.py`:
- Line 80: The code uses Path.write_text without specifying encoding (calls to
f.write_text(text) and f.write_text(new_text)), which may re-encode Fortran
source files with the platform default; update both occurrences in
docs/fix_file_briefs.py to call write_text with an explicit encoding (e.g.,
encoding='utf-8') so files are written with a stable encoding rather than the
platform default.
- Line 41: Add "from __future__ import annotations" at the top of the module so
annotations like the return type of find_entity (tuple[str, str] | None) are
evaluated lazily and remain compatible with Python 3.7–3.9, and update the two
write_text(...) calls (the ones modifying Fortran source) to pass
encoding='utf-8' to ensure explicit round-trip encoding when writing files.
In `@docs/gen_api_landing.py`:
- Line 91: The call to out.write_text("\n".join(lines)) uses the platform
default encoding; update the Path.write_text invocation (out.write_text) to pass
encoding='utf-8' so the file is written deterministically across platforms
(i.e., out.write_text("\n".join(lines), encoding="utf-8")). Ensure the joined
content is a str before writing to avoid encoding issues.
- Around line 48-52: The function get_modules uses PEP 585 typing (list[str])
which breaks on Python 3.8; add the line "from __future__ import annotations" at
the top of the file (before any other imports) to enable postponed evaluation of
annotations so get_modules and any other annotations using built-in generics
work on Python 3.8–3.9 environments.
In `@docs/simulation/readme.md`:
- Around line 41-54: gen_api_landing.py currently only scans *.fpp files and
omits four public .f90 modules; update the scanner logic in gen_api_landing.py
to include .f90 files (or explicitly include m_precision_select.f90,
m_compile_specific.f90, m_delay_file_access.f90, m_nvtx.f90) when collecting
modules for the API landing page, ensure the parser/regex that detects Doxygen
headers and module names handles .f90 syntax, and add tests or a small fixture
to confirm m_precision_select, m_compile_specific, m_delay_file_access, and
m_nvtx appear in the generated landing pages.
---
Outside diff comments:
In `@src/pre_process/m_check_ib_patches.fpp`:
- Around line 302-303: The Doxygen opener for s_check_inactive_ib_patch_geometry
is malformed: replace the leading "!!>" token with the correct Doxygen block
opener used elsewhere (use "!!<" as the opener) so the subroutine docblock is
recognized—update the comment header for s_check_inactive_ib_patch_geometry to
use "!!<" and match the style used in m_chemistry.fpp/m_helper_basic.fpp
(preserve the rest of the comment text and spacing).
In `@src/simulation/m_surface_tension.fpp`:
- Line 39: Replace the malformed Doxygen group-close marker "!> @)" with the
correct "!> @}" in the Fortran module comments so the "@{" group opened earlier
is properly closed; locate the comment line containing the exact token "!> @)"
in m_surface_tension.fpp and update it to "!> @}" (no other code changes
required).
---
Nitpick comments:
In `@CMakeLists.txt`:
- Around line 811-839: The stamp targets gen-api-landing.stamp and
fix-file-briefs.stamp only depend on their Python scripts so adding/renaming
.fpp modules won't retrigger them; update the add_custom_command DEPENDS for the
gen_api_landing and fix_file_briefs commands to include the globbed Fortran
source list (e.g. ${api_fpp_srcs} or the appropriate .fpp glob variable you use
in this CMakeLists) so any .fpp change invalidates the stamp, and apply the
identical change to the fix-file-briefs command to ensure both stamps regenerate
when .fpp sources change.
In `@docs/gen_api_landing.py`:
- Line 71: Replace the fragile title-splitting logic that builds section
headings from info['title'].split()[-1] with code that first looks for an
explicit 'section_title' key on the target metadata (i.e., in the TARGETS dict /
the info dict used here) and uses that if present; if no 'section_title' is
provided, fall back to the existing split()[-1] behavior for backward
compatibility. Update the place that appends the heading (the lines.append call
in docs/gen_api_landing.py) to use info.get('section_title',
info['title'].split()[-1]) so maintainers can set explicit section names without
breaking existing titles.
In `@docs/index.html`:
- Line 175: The grid at the div with class "justify-center grid grid-cols-2
sm:grid-cols-3 lg:grid-cols-7 gap-x-4 md:gap-x-8 text-md md:text-xl text-center
text-white font-medium" creates an orphaned single item at the sm breakpoint;
update the class to either replace sm:grid-cols-3 with sm:grid-cols-4 (so items
lay out 4+3) or keep sm:grid-cols-3 and add justify-items-center to the class
list to center the lone item—apply the change to that div's class attribute.
- Around line 195-198: The "API" nav anchor (the <a> element with
href="simulation/index.html" and inner <span>API</span>) currently points only
to the Simulation landing; update navigation so the "API" link goes to a
dedicated API index that lists Pre-Process, Simulation, and Post-Process (create
a new api/index.html that contains links to each target), and change the
anchor's href from "simulation/index.html" to "api/index.html" (alternatively
implement a dropdown menu from that anchor that exposes the three targets).
Ensure the anchor text remains "API" so users land directly on a page listing
all three API targets.
- Add DOXYGEN_WARN_LOGFILE to CMakeLists.txt and Doxyfile.in to capture per-target warning logs during documentation builds - Fix stale, missing, duplicate, and misspelled @param entries across 28 source files (common, pre_process, post_process, simulation modules) - Common module fixes (m_helper, m_model, m_mpi_common, m_phase_change, m_finite_differences) eliminate 3x warnings since each is shared across all three Doxygen targets - Remaining 5 warnings are structural unbalanced grouping from fypp macro expansion, not @param issues Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
bot
added
the
size:XL
Nitpicks 🔍
|
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| # Scans src/{target}/*.fpp to produce module lists in docs/{target}/readme.md. | ||
| add_custom_command( | ||
| OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/gen-api-landing.stamp" | ||
| DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/docs/gen_api_landing.py" |
There was a problem hiding this comment.
Suggestion: Extend the gen_api_landing custom command dependencies to also include the Fortran template sources it scans, so API landing pages regenerate when those .fpp files change. [custom_rule]
Severity Level: Minor
| DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/docs/gen_api_landing.py" | |
| ${pre_process_FPPs} | |
| ${simulation_FPPs} | |
| ${post_process_FPPs} |
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** CMakeLists.txt
**Line:** 815:815
**Comment:**
*Custom Rule: Extend the `gen_api_landing` custom command dependencies to also include the Fortran template sources it scans, so API landing pages regenerate when those `.fpp` files change.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| add_custom_command( | ||
| OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/fix-file-briefs.stamp" | ||
| DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/docs/fix_file_briefs.py" | ||
| COMMAND "${Python3_EXECUTABLE}" "${CMAKE_CURRENT_SOURCE_DIR}/docs/fix_file_briefs.py" |
There was a problem hiding this comment.
Suggestion: Extend the fix_file_briefs custom command dependencies to include the Fortran template sources whose module and program declarations it inspects, so header fixes rerun when those sources change. [custom_rule]
Severity Level: Minor
| COMMAND "${Python3_EXECUTABLE}" "${CMAKE_CURRENT_SOURCE_DIR}/docs/fix_file_briefs.py" | |
| ${pre_process_FPPs} | |
| ${simulation_FPPs} | |
| ${post_process_FPPs} |
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** CMakeLists.txt
**Line:** 832:832
**Comment:**
*Custom Rule: Extend the `fix_file_briefs` custom command dependencies to include the Fortran template sources whose module and program declarations it inspects, so header fixes rerun when those sources change.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| - @ref authors "Authors" - Contributors | ||
|
|
||
|
|
||
| <div style='text-align:center; font-size:0.75rem; color:#888; padding:16px 0 0;'>Page last updated: 2026-02-19</div> |
There was a problem hiding this comment.
Suggestion: Replace the hard-coded "last updated" date in the footer with wording that does not require manual date maintenance. [custom_rule]
Severity Level: Minor
| <div style='text-align:center; font-size:0.75rem; color:#888; padding:16px 0 0;'>Page last updated: 2026-02-19</div> | |
| <div style='text-align:center; font-size:0.75rem; color:#888; padding:16px 0 0;'>See the repository history for the latest updates to this page.</div> |
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** docs/documentation/readme.md
**Line:** 46:46
**Comment:**
*Custom Rule: Replace the hard-coded "last updated" date in the footer with wording that does not require manual date maintenance.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
docs/fix_file_briefs.py
Outdated
|
|
||
| # First `module X` or `program X` that isn't `end module/program`. | ||
| DECL_RE = re.compile( | ||
| r"^\s*(module|program)\s+(\w+)\s*$", re.MULTILINE | re.IGNORECASE |
There was a problem hiding this comment.
Suggestion: Refine the declaration regex so it does not treat module procedure lines or similar constructs as top-level modules. [custom_rule]
Severity Level: Minor
| r"^\s*(module|program)\s+(\w+)\s*$", re.MULTILINE | re.IGNORECASE | |
| r"^\s*(module(?!\s+procedure)\b|program)\s+(\w+)\s*$", | |
| re.MULTILINE | re.IGNORECASE, |
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** docs/fix_file_briefs.py
**Line:** 30:30
**Comment:**
*Custom Rule: Refine the declaration regex so it does not treat `module procedure` lines or similar constructs as top-level modules.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
docs/fix_file_briefs.py
Outdated
| text = f.read_text() | ||
| entity = find_entity(text) | ||
| if entity is None: | ||
| continue | ||
| kind, name = entity | ||
| ref = make_ref(name) | ||
|
|
||
| if not has_file_directive(text): | ||
| # No @file at all — prepend a complete header. | ||
| header = f"!>\n!! @file\n!! @brief Contains {kind} {ref}\n\n" | ||
| text = header + text | ||
| f.write_text(text) | ||
| fixed += 1 | ||
| print(f"Added {f.relative_to(src_dir)}") | ||
| continue | ||
|
|
||
| # Has @file — check if there's a "Contains module/program" brief to fix. | ||
| m = BRIEF_CONTAINS_RE.search(text) | ||
| if m is None: | ||
| continue # Has @file but no "Contains ..." brief — leave it alone | ||
|
|
||
| current_name = m.group(3).strip() | ||
| if current_name == ref: | ||
| continue # Already correct | ||
|
|
||
| # Replace the name portion of the brief line. | ||
| new_line = f"{m.group(1)} @brief {m.group(2)}{ref}" | ||
| new_text = text[: m.start()] + new_line + text[m.end() :] | ||
|
|
||
| if new_text != text: | ||
| f.write_text(new_text) |
There was a problem hiding this comment.
Suggestion: Read and write Fortran sources using an explicit UTF-8 encoding to avoid locale-dependent behavior on different systems. [custom_rule]
Severity Level: Minor
| text = f.read_text() | |
| entity = find_entity(text) | |
| if entity is None: | |
| continue | |
| kind, name = entity | |
| ref = make_ref(name) | |
| if not has_file_directive(text): | |
| # No @file at all — prepend a complete header. | |
| header = f"!>\n!! @file\n!! @brief Contains {kind} {ref}\n\n" | |
| text = header + text | |
| f.write_text(text) | |
| fixed += 1 | |
| print(f"Added {f.relative_to(src_dir)}") | |
| continue | |
| # Has @file — check if there's a "Contains module/program" brief to fix. | |
| m = BRIEF_CONTAINS_RE.search(text) | |
| if m is None: | |
| continue # Has @file but no "Contains ..." brief — leave it alone | |
| current_name = m.group(3).strip() | |
| if current_name == ref: | |
| continue # Already correct | |
| # Replace the name portion of the brief line. | |
| new_line = f"{m.group(1)} @brief {m.group(2)}{ref}" | |
| new_text = text[: m.start()] + new_line + text[m.end() :] | |
| if new_text != text: | |
| f.write_text(new_text) | |
| text = f.read_text(encoding="utf-8") | |
| entity = find_entity(text) | |
| if entity is None: | |
| continue | |
| kind, name = entity | |
| ref = make_ref(name) | |
| if not has_file_directive(text): | |
| # No @file at all — prepend a complete header. | |
| header = f"!>\n!! @file\n!! @brief Contains {kind} {ref}\n\n" | |
| text = header + text | |
| f.write_text(text, encoding="utf-8") | |
| fixed += 1 | |
| print(f"Added {f.relative_to(src_dir)}") | |
| continue | |
| # Has @file — check if there's a "Contains module/program" brief to fix. | |
| m = BRIEF_CONTAINS_RE.search(text) | |
| if m is None: | |
| continue # Has @file but no "Contains ..." brief — leave it alone | |
| current_name = m.group(3).strip() | |
| if current_name == ref: | |
| continue # Already correct | |
| # Replace the name portion of the brief line. | |
| new_line = f"{m.group(1)} @brief {m.group(2)}{ref}" | |
| new_text = text[: m.start()] + new_line + text[m.end() :] | |
| if new_text != text: | |
| f.write_text(new_text, encoding="utf-8") |
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** docs/fix_file_briefs.py
**Line:** 69:99
**Comment:**
*Custom Rule: Read and write Fortran sources using an explicit UTF-8 encoding to avoid locale-dependent behavior on different systems.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| !> | ||
| !! @file | ||
| !! @brief Contains module m_bubbles_EL_kernels | ||
| !! @brief Contains module @ref m_bubbles_el_kernels "m_bubbles_EL_kernels" |
There was a problem hiding this comment.
Suggestion: Update the module reference in the brief tag so it exactly matches the declared module name, ensuring documentation tools can resolve the link correctly. [custom_rule]
Severity Level: Minor
| !! @brief Contains module @ref m_bubbles_el_kernels "m_bubbles_EL_kernels" | |
| !! @brief Contains module @ref m_bubbles_EL_kernels "m_bubbles_EL_kernels" |
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/simulation/m_bubbles_EL_kernels.fpp
**Line:** 3:3
**Comment:**
*Custom Rule: Update the module reference in the brief tag so it exactly matches the declared module name, ensuring documentation tools can resolve the link correctly.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| !! @param q_prim_vf Primitive variables | ||
| !! @param pb Internal bubble pressure | ||
| !! @param mv Mass of vapor in bubble | ||
| !! @param pb_in Internal bubble pressure |
There was a problem hiding this comment.
Suggestion: Add back parameter documentation lines for q_cons_vf and q_prim_vf so that all arguments of the subroutine are described alongside the new pb_in and mv_in docs. [custom_rule]
Severity Level: Minor
| !! @param pb_in Internal bubble pressure | |
| !! @param q_cons_vf Conservative variables | |
| !! @param q_prim_vf Primitive variables |
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/simulation/m_ibm.fpp
**Line:** 159:159
**Comment:**
*Custom Rule: Add back parameter documentation lines for `q_cons_vf` and `q_prim_vf` so that all arguments of the subroutine are described alongside the new `pb_in` and `mv_in` docs.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.|
|
||
| !> Function that uses the interpolation coefficients and the current state | ||
| !! at the cell centers in order to estimate the state at the image point | ||
| !! @param gp Ghost point data structure |
There was a problem hiding this comment.
Suggestion: Add a parameter documentation line for q_prim_vf so the first argument of the image-point interpolation routine is also described. [custom_rule]
Severity Level: Minor
| !! @param gp Ghost point data structure | |
| !! @param q_prim_vf Primitive variables at cell centers |
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/simulation/m_ibm.fpp
**Line:** 854:854
**Comment:**
*Custom Rule: Add a parameter documentation line for `q_prim_vf` so the first argument of the image-point interpolation routine is also described.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.|
|
||
| !> Computes the moment of inertia for an immersed boundary | ||
| !! @param ib_marker Immersed boundary marker index | ||
| subroutine s_compute_moment_of_inertia(ib_marker, axis) |
There was a problem hiding this comment.
Suggestion: Add a parameter documentation line for axis so both arguments of the moment-of-inertia routine are described. [custom_rule]
Severity Level: Minor
| subroutine s_compute_moment_of_inertia(ib_marker, axis) | |
| !! @param axis Rotation axis vector about which the moment is computed |
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/simulation/m_ibm.fpp
**Line:** 1265:1265
**Comment:**
*Custom Rule: Add a parameter documentation line for `axis` so both arguments of the moment-of-inertia routine are described.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
|
CodeAnt AI finished reviewing your PR. |
- m_surface_tension.fpp: typo @) -> @} (close group)
- m_global_parameters.fpp (sim): copy-paste @{ -> @} (close group)
- m_global_parameters.fpp (post): typo #} -> @} (close group)
- m_rhs.fpp: add missing @} closer for index bounds group
- m_compute_levelset.fpp: remove orphaned @} with no matching opener
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/simulation/m_bubbles.fpp (1)
318-327:⚠️ Potential issue | 🟡 Minor
s_bwpropertydocstring is missing@paramentries for its three output parameters.The
intent(out)parameterschi_vw_out,k_mw_out, andrho_mw_out(lines 325–327) have no corresponding@paramlines in the Doxygen block, yet their inline comments are already present. Given that this PR focuses on documentation completeness, these should be added.📝 Proposed addition
!> Subroutine that computes bubble wall properties for vapor bubbles !! `@param` pb_in Internal bubble pressure !! `@param` iR0 Current bubble size index + !! `@param` chi_vw_out Mass fraction of vapour at the bubble wall (Ando 2010) + !! `@param` k_mw_out Thermal conductivity of the gas mixture at the bubble wall (Ando 2010) + !! `@param` rho_mw_out Density of the gas mixture at the bubble wall (Ando 2010) elemental subroutine s_bwproperty(pb_in, iR0, chi_vw_out, k_mw_out, rho_mw_out)Based on learnings: "Add Doxygen docstrings in source files for new public routines" — applied here to the output parameters of an existing routine being actively edited in this PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/simulation/m_bubbles.fpp` around lines 318 - 327, The Doxygen block for subroutine s_bwproperty is missing `@param` entries for the three output arguments; add `@param` lines documenting chi_vw_out, k_mw_out, and rho_mw_out (including brief descriptions matching their inline comments such as "Bubble wall properties (Ando 2010)") to the existing docstring above elemental subroutine s_bwproperty so all intent(out) parameters are documented.
🧹 Nitpick comments (7)
src/simulation/m_time_steppers.fpp (1)
767-779: Missing@param ldtin the Doxygen block fors_apply_bodyforces.Three of the four parameters are now documented via
@param, butldtis absent from the block comment. It only has a terse inline!< local dtannotation (Line 778). Since the explicit goal of this PR is complete API documentation for these routines, the block is incomplete.📝 Proposed fix
!> This subroutine applies the body forces source term at each !! Runge-Kutta stage !! `@param` q_cons_vf Conservative variables !! `@param` q_prim_vf_in Primitive variables !! `@param` rhs_vf_in Right-hand side variables + !! `@param` ldt Local time-step size (scaled RK coefficient * dt) subroutine s_apply_bodyforces(q_cons_vf, q_prim_vf_in, rhs_vf_in, ldt)Based on learnings: "Add Doxygen docstrings in source files for new public routines."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/simulation/m_time_steppers.fpp` around lines 767 - 779, The Doxygen block for subroutine s_apply_bodyforces is missing documentation for the ldt parameter; update the routine's header comment to include an `@param` ldt description (e.g., "@param ldt local time step") so all four parameters (q_cons_vf, q_prim_vf_in, rhs_vf_in, ldt) are documented consistently with the existing `@param` entries and the inline "!< local dt" annotation can remain or be removed as desired.src/post_process/m_data_input.f90 (1)
146-148:@param t_stepdescription should indicate the argument is optional.
t_stepis declaredinteger, intent(in), optional(line 152), but the new@paramline simply says "Time step index". Readers of the generated API docs won't know it can be omitted, nor under what condition it is required (i.e., whenparallel_iois.true.). A more accurate description prevents misuse.📝 Suggested doc fix
-!! `@param` t_step Time step index +!! `@param` t_step (Optional) Time step index; required when parallel_io is .true.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/post_process/m_data_input.f90` around lines 146 - 148, Update the `@param` documentation for the argument t_step in the helper subroutine that reads IB data files to state that t_step is optional (matches the declaration integer, intent(in), optional) and clarify its requirement: explicitly note that t_step may be omitted unless parallel_io is .true;, in which case a time step index must be provided; reference the t_step parameter and the parallel_io flag in the description so generated API docs and readers understand when the argument is required.src/simulation/m_hyperelastic.fpp (1)
217-228: Stale description body in both solver subroutines — describess_hyperelastic_rmt_stress_update, not the Cauchy solversThe boilerplate lines retained in both solver comment blocks:
!! calculate the grad_xi, grad_xi is a nxn tensor !! calculate the inverse of grad_xi to obtain F, F is a nxn tensor !! calculate the FFtranspose to obtain the btensor, btensor is nxn tensor !! btensor is symmetric, save the data spacedescribe the deformation-gradient / btensor computation pipeline that lives in
s_hyperelastic_rmt_stress_update. These solvers receive an already-computedbtensor_inand compute Cauchy stresses from it — they do none of the grad_xi / inverse / F·F^T work. Since this PR is explicitly improving API documentation, replacing these lines with a brief, accurate description of what each solver actually does (compute deviatoric btensor and update stress primitive fields via neo-Hookean / Mooney-Rivlin EOS) would prevent generated API docs from describing the wrong algorithm.Also applies to: 260-271
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/simulation/m_hyperelastic.fpp` around lines 217 - 228, The comment block incorrectly describes deformation-gradient and btensor construction (grad_xi, inverse, F·F^T) which is done in s_hyperelastic_rmt_stress_update; update the doc for the solver subroutines (referencing btensor_in, q_prim_vf, G_param and the solver names) to state that these routines accept a precomputed btensor_in, compute the deviatoric part of btensor and then compute/update the Cauchy stress / primitive stress fields using the appropriate constitutive law (neo-Hookean or Mooney-Rivlin), explicitly noting they do not compute grad_xi or F.CMakeLists.txt (1)
813-822: Add the globbed.fppfile lists as dependencies so the landing pages rebuild when modules change.The script reads
src/{target}/*.fppandsrc/common/*.fppto generate module documentation, but these files are not declared as dependencies. Adding the already-globbed lists will ensure the build re-runs when you add, remove, or rename a module.Proposed fix
add_custom_command( OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/gen-api-landing.stamp" DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/docs/gen_api_landing.py" + "${pre_process_FPPs}" "${simulation_FPPs}" "${post_process_FPPs}" COMMAND "${Python3_EXECUTABLE}" "${CMAKE_CURRENT_SOURCE_DIR}/docs/gen_api_landing.py" "${CMAKE_CURRENT_SOURCE_DIR}" COMMAND "${CMAKE_COMMAND}" -E touch "${CMAKE_CURRENT_BINARY_DIR}/gen-api-landing.stamp" COMMENT "Generating API landing pages" VERBATIM )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CMakeLists.txt` around lines 813 - 822, The add_custom_command that produces "${CMAKE_CURRENT_BINARY_DIR}/gen-api-landing.stamp" must include the globbed .fpp inputs so changes to modules trigger reruns; update the add_custom_command DEPENDS to include the previously-globbed lists used for src/{target}/*.fpp and src/common/*.fpp (the same variables or file(GLOB ...) expressions used elsewhere), ensuring the script "${CMAKE_CURRENT_SOURCE_DIR}/docs/gen_api_landing.py" and its generated stamp target (gen_api_landing / gen-api-landing.stamp) also depend on those .fpp files so CMake will rebuild when .fpp files are added/removed/renamed.src/simulation/m_sim_helpers.fpp (1)
96-96: Inconsistent capitalization in new@paramdescriptions.All three new
@paramentries (lines 96, 172, 239) capitalize the first word of the description ("Vapor quality", "Density", "Speed of sound"), while every pre-existing@paramin this file uses lowercase ("directional velocities", "mixture pressure", "cell centered maximum dt", etc.). For consistency:📝 Proposed capitalization fix
- !! `@param` qv Vapor quality + !! `@param` qv vapor quality (or preferred: mixture reference internal energy)- !! `@param` rho Density + !! `@param` rho density- !! `@param` c Speed of sound + !! `@param` c speed of soundAlso applies to: 172-172, 239-239
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/simulation/m_sim_helpers.fpp` at line 96, The three newly added `@param` descriptions use capitalized first words; update them to match the file's existing style by lowercasing the first word for each parameter: change the `@param` for qv from "Vapor quality" to "vapor quality", the `@param` for rho from "Density" to "density", and the `@param` for cs (or the parameter documenting speed of sound) from "Speed of sound" to "speed of sound" so the docblock capitalization is consistent with other `@param` entries.src/simulation/m_viscous.fpp (1)
554-562:s_get_viscousnow has 21 parameters — consider a derived-type bundle.The signature grew to 21 arguments (3 left rsx/rsy/rsz arrays, 3 left derivative arrays, 1 qL_prim, and 7 right-side mirrors, plus q_prim_qp, 3 dq_prim derivatives, and ix/iy/iz). The project guideline caps routines at 6 arguments and recommends a derived-type params struct beyond that.
Grouping the left/right boundary and derivative fields into a small bundle type (e.g.,
boundary_prim_data) would halve the argument list and make future directional expansions local to the type definition.Based on learnings, the project guideline states: "Prefer ≤ 6 subroutine arguments; consider a derived-type params struct for more."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/simulation/m_viscous.fpp` around lines 554 - 562, s_get_viscous currently takes 21 arguments which exceeds the project guideline; refactor by creating one or more derived types (e.g., boundary_prim_data for rsx/rsy/rsz and their derivatives, and maybe qprim_point for q_prim_qp and dq_prim_* ) and replace the long parameter list with a few typed bundles passed to s_get_viscous (and updated callers). Update the subroutine signature (s_get_viscous) to accept these derived-type objects plus ix, iy, iz, move related declarations into the types, and adjust all call sites to construct and pass the new bundles instead of the individual arrays.src/simulation/m_bubbles_EL.fpp (1)
845-846:@param preterm1description "Pre-computed term 1" is slightly misleading.
preterm1andterm2are bothintent(out), optional— they are computed insides_get_pinfand returned to the caller. "Pre-computed" implies the caller provides a pre-computed value (like anintent(in)argument), which inverts the data-flow direction.💡 Suggested wording
- !! `@param` preterm1 Pre-computed term 1 - !! `@param` term2 Computed term 2 + !! `@param` preterm1 Output coefficient: prefactor for the pressure-correction term (3/2 * Rbeq * (dc²-Rbeq²) / (aux * denom)) + !! `@param` term2 Output correction: velocity-weighted radial term for the subgrid pressure model🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/simulation/m_bubbles_EL.fpp` around lines 845 - 846, The documentation for s_get_pinf incorrectly labels preterm1 as "Pre-computed term 1" and thus implies caller-provided input; update the comment for parameters preterm1 and term2 to reflect that both are intent(out), optional values computed inside s_get_pinf and returned to the caller (e.g., "Computed term 1 (output, optional) returned by s_get_pinf"); ensure the description matches the intent(out) semantics for preterm1 and term2.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CMakeLists.txt`:
- Around line 829-838: The custom CMake commands (add_custom_command for the
fix_file_briefs target and the gen_api_landing block) only depend on the Python
script and therefore won't re-run when FPP source files change; update the
DEPENDS lists of add_custom_command (the one creating fix-file-briefs.stamp) and
the gen_api_landing add_custom_command to include the FPP source file variables
"${pre_process_FPPs}", "${simulation_FPPs}", and "${post_process_FPPs}" so
edits/renames in .fpp/.f90 trigger the script and regenerate the stamp/target.
In `@docs/documentation/visualization.md`:
- Line 145: Replace the hardcoded footer div that contains "Page last updated:
2026-02-13" with a build-time injected value: locate the HTML snippet "<div
style='text-align:center; font-size:0.75rem; color:`#888`; padding:16px 0 0;'>Page
last updated:" and change it to use your static-site templating variable or
plugin (e.g., MkDocs/Sphinx git-revision-date variable or a git-based per-file
date like `git log -1 --format=%ci -- <file>`) so the date is generated at build
time for each page rather than hardcoded; ensure the templating syntax you use
is compatible with the site generator in use and update any other occurrences of
the same footer across the docs to the same templated approach.
In `@src/common/m_variables_conversion.fpp`:
- Around line 1182-1183: The parameters s2b and s3b are documented but unused:
either remove them (and their docs) from the subroutine signature and related
documentation, or wire them into the loop bounds by replacing the hardcoded
assignments is2b = is2%beg and is3b = is3%beg with is2b = s2b and is3b = s3b so
the routine honors the provided starting boundary indices; update any call sites
if you remove parameters and ensure consistency in the subroutine (variables:
s2b, s3b, is2b, is3b, is2%beg, is3%beg).
In `@src/simulation/m_bubbles_EE.fpp`:
- Around line 95-97: Add a `@param` entry for divu_in to the Doxygen docblock for
s_compute_bubbles_EE_rhs so the third parameter is documented (the inline `!<`
comment at line 102 is not picked up by Doxygen); update the comment block that
currently lists `@param` idir and `@param` q_prim_vf to include `@param divu_in`
with a short description of what divu_in represents (e.g., divergence of
velocity) so the generated parameter table is complete.
- Line 162: The function s_compute_bubble_EE_source has a missing `@param` entry
for the fourth parameter divu_in in its Doxygen block (you added `@param` rhs_vf
but left divu_in only as an inline !<), so update the docstring for
s_compute_bubble_EE_source to include a proper "@param divu_in" line in the
block header with a short description of what divu_in represents/contains;
ensure this matches the existing style used for "@param rhs_vf" so Doxygen will
include it in the generated parameter table.
In `@src/simulation/m_bubbles.fpp`:
- Around line 319-321: The Doxygen block for elemental subroutine s_bwproperty
is missing `@param` entries for its output arguments; update the docstring for
s_bwproperty to include the three output parameters chi_vw_out, k_mw_out, and
rho_mw_out with brief descriptions (e.g., "Bubble wall properties (Ando 2010)"),
matching the style used for other routines like f_rddot so all parameters are
documented in the `@param` block.
In `@src/simulation/m_hyperelastic.fpp`:
- Around line 219-224: Update the parameter documentation to reflect mutation:
change the `@param` entries for btensor_in and q_prim_vf to include the direction
qualifier [in,out] in the docblocks for both s_neoHookean_cauchy_solver and
s_Mooney_Rivlin_cauchy_solver (and the duplicate docblock at lines referenced
around 262–267), since btensor_in and q_prim_vf are declared intent(inout) and
are modified in-place (diagonal overwritten and stress/invariants written);
ensure the two `@param` lines read e.g. "@param btensor_in [in,out] Left
Cauchy-Green deformation tensor" and "@param q_prim_vf [in,out] Primitive
variables" so callers and readers see the correct contract.
In `@src/simulation/m_ibm.fpp`:
- Around line 852-869: The docblock for subroutine s_interpolate_image_point is
missing a `@param` entry for the first argument q_prim_vf; update the comment
header to add a concise `@param` description for q_prim_vf matching its
declaration (type(scalar_field), dimension(sys_size), intent(IN)) and purpose
(e.g., primary field values at cell centers used for interpolation), placing it
alongside the existing `@param` entries so the parameter list is complete and
consistent with the subroutine signature.
- Around line 1263-1265: The docblock for subroutine s_compute_moment_of_inertia
is missing a `@param` entry for the second argument axis and the axis
normalization is wrong; update the comment to include "@param axis the axis
about which we compute the moment (only required in 3D)" and change the
normalization logic for normal_axis in s_compute_moment_of_inertia to divide by
sqrt(sum(axis**2)) (keeping the existing guard that checks sum(axis**2) > 0) so
the axis is normalized by its Euclidean norm rather than sqrt(sum(axis)).
In `@src/simulation/m_sim_helpers.fpp`:
- Line 96: Update the doc comment for the parameter qv in the m_sim_helpers.fpp
comment block: replace the description "Vapor quality" with "Fluid reference
energy" so it matches the terminology used across the codebase (e.g.,
m_variables_conversion, m_time_steppers, m_data_output, m_cbc) and correctly
reflects that qv is the equation-of-state reference internal energy term used in
the E and pres expressions.
---
Outside diff comments:
In `@src/simulation/m_bubbles.fpp`:
- Around line 318-327: The Doxygen block for subroutine s_bwproperty is missing
`@param` entries for the three output arguments; add `@param` lines documenting
chi_vw_out, k_mw_out, and rho_mw_out (including brief descriptions matching
their inline comments such as "Bubble wall properties (Ando 2010)") to the
existing docstring above elemental subroutine s_bwproperty so all intent(out)
parameters are documented.
---
Nitpick comments:
In `@CMakeLists.txt`:
- Around line 813-822: The add_custom_command that produces
"${CMAKE_CURRENT_BINARY_DIR}/gen-api-landing.stamp" must include the globbed
.fpp inputs so changes to modules trigger reruns; update the add_custom_command
DEPENDS to include the previously-globbed lists used for src/{target}/*.fpp and
src/common/*.fpp (the same variables or file(GLOB ...) expressions used
elsewhere), ensuring the script
"${CMAKE_CURRENT_SOURCE_DIR}/docs/gen_api_landing.py" and its generated stamp
target (gen_api_landing / gen-api-landing.stamp) also depend on those .fpp files
so CMake will rebuild when .fpp files are added/removed/renamed.
In `@src/post_process/m_data_input.f90`:
- Around line 146-148: Update the `@param` documentation for the argument t_step
in the helper subroutine that reads IB data files to state that t_step is
optional (matches the declaration integer, intent(in), optional) and clarify its
requirement: explicitly note that t_step may be omitted unless parallel_io is
.true;, in which case a time step index must be provided; reference the t_step
parameter and the parallel_io flag in the description so generated API docs and
readers understand when the argument is required.
In `@src/simulation/m_bubbles_EL.fpp`:
- Around line 845-846: The documentation for s_get_pinf incorrectly labels
preterm1 as "Pre-computed term 1" and thus implies caller-provided input; update
the comment for parameters preterm1 and term2 to reflect that both are
intent(out), optional values computed inside s_get_pinf and returned to the
caller (e.g., "Computed term 1 (output, optional) returned by s_get_pinf");
ensure the description matches the intent(out) semantics for preterm1 and term2.
In `@src/simulation/m_hyperelastic.fpp`:
- Around line 217-228: The comment block incorrectly describes
deformation-gradient and btensor construction (grad_xi, inverse, F·F^T) which is
done in s_hyperelastic_rmt_stress_update; update the doc for the solver
subroutines (referencing btensor_in, q_prim_vf, G_param and the solver names) to
state that these routines accept a precomputed btensor_in, compute the
deviatoric part of btensor and then compute/update the Cauchy stress / primitive
stress fields using the appropriate constitutive law (neo-Hookean or
Mooney-Rivlin), explicitly noting they do not compute grad_xi or F.
In `@src/simulation/m_sim_helpers.fpp`:
- Line 96: The three newly added `@param` descriptions use capitalized first
words; update them to match the file's existing style by lowercasing the first
word for each parameter: change the `@param` for qv from "Vapor quality" to "vapor
quality", the `@param` for rho from "Density" to "density", and the `@param` for cs
(or the parameter documenting speed of sound) from "Speed of sound" to "speed of
sound" so the docblock capitalization is consistent with other `@param` entries.
In `@src/simulation/m_time_steppers.fpp`:
- Around line 767-779: The Doxygen block for subroutine s_apply_bodyforces is
missing documentation for the ldt parameter; update the routine's header comment
to include an `@param` ldt description (e.g., "@param ldt local time step") so all
four parameters (q_cons_vf, q_prim_vf_in, rhs_vf_in, ldt) are documented
consistently with the existing `@param` entries and the inline "!< local dt"
annotation can remain or be removed as desired.
In `@src/simulation/m_viscous.fpp`:
- Around line 554-562: s_get_viscous currently takes 21 arguments which exceeds
the project guideline; refactor by creating one or more derived types (e.g.,
boundary_prim_data for rsx/rsy/rsz and their derivatives, and maybe qprim_point
for q_prim_qp and dq_prim_* ) and replace the long parameter list with a few
typed bundles passed to s_get_viscous (and updated callers). Update the
subroutine signature (s_get_viscous) to accept these derived-type objects plus
ix, iy, iz, move related declarations into the types, and adjust all call sites
to construct and pass the new bundles instead of the individual arrays.
| Once this is done, click `configure` and then `save` on the next dialogue box. | ||
|
|
||
|
|
||
| <div style='text-align:center; font-size:0.75rem; color:#888; padding:16px 0 0;'>Page last updated: 2026-02-13</div> |
There was a problem hiding this comment.
Hardcoded date will silently become stale.
2026-02-13 is static HTML embedded in 16+ docs pages across this PR. Any future content edit won't update these footers, and there's no mechanism to detect the drift. If the site is built via a static generator (e.g., MkDocs/Sphinx), consider injecting the date at build time (e.g., the git log -1 --format=%ci -- <file> per-file approach, or a MkDocs git-revision-date-localized plugin) rather than hardcoding it.
— The AI summary states the timestamp is "effectively duplicated at two locations within the page," but the complete file contains only one instance of the footer (line 145).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/documentation/visualization.md` at line 145, Replace the hardcoded
footer div that contains "Page last updated: 2026-02-13" with a build-time
injected value: locate the HTML snippet "<div style='text-align:center;
font-size:0.75rem; color:`#888`; padding:16px 0 0;'>Page last updated:" and change
it to use your static-site templating variable or plugin (e.g., MkDocs/Sphinx
git-revision-date variable or a git-based per-file date like `git log -1
--format=%ci -- <file>`) so the date is generated at build time for each page
rather than hardcoded; ensure the templating syntax you use is compatible with
the site generator in use and update any other occurrences of the same footer
across the docs to the same templated approach.
| !! @param s2b Starting boundary index in the second coordinate direction | ||
| !! @param s3b Starting boundary index in the third coordinate direction |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n '\bs[23]b\b' src/common/m_variables_conversion.fppRepository: MFlowCode/MFC
Length of output: 357
🏁 Script executed:
sed -n '1150,1250p' src/common/m_variables_conversion.fpp | cat -nRepository: MFlowCode/MFC
Length of output: 5210
🏁 Script executed:
sed -n '1220,1230p' src/common/m_variables_conversion.fpp | cat -nRepository: MFlowCode/MFC
Length of output: 527
🏁 Script executed:
# Get the full subroutine to search for s2b/s3b usage within the body
sed -n '1175,1350p' src/common/m_variables_conversion.fpp | grep -E '(subroutine|end subroutine|s[23]b)' | head -40Repository: MFlowCode/MFC
Length of output: 809
🏁 Script executed:
# Find the exact line range of the subroutine and verify no other s2b/s3b usage
awk '/subroutine s_convert_primitive_to_flux_variables/,/end subroutine s_convert_primitive_to_flux_variables/ {print NR": "$0}' src/common/m_variables_conversion.fpp | grep -E '(subroutine|end subroutine|s[23]b)'Repository: MFlowCode/MFC
Length of output: 549
Remove unused parameters s2b and s3b or implement their intended functionality.
These parameters are declared and documented at lines 1182–1189 but never used in the subroutine body. Loop indices is2b and is3b are hardcoded to is2%beg and is3%beg (lines 1224–1225), not derived from s2b/s3b. Either remove the dead parameters and their documentation, or if they were intended to allow partial-domain GPU kernels, assign is2b = s2b and is3b = s3b instead. Since this is in src/common/, changes affect all three executables.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/common/m_variables_conversion.fpp` around lines 1182 - 1183, The
parameters s2b and s3b are documented but unused: either remove them (and their
docs) from the subroutine signature and related documentation, or wire them into
the loop bounds by replacing the hardcoded assignments is2b = is2%beg and is3b =
is3%beg with is2b = s2b and is3b = s3b so the routine honors the provided
starting boundary indices; update any call sites if you remove parameters and
ensure consistency in the subroutine (variables: s2b, s3b, is2b, is3b, is2%beg,
is3%beg).
| !> Compute the right-hand side for Euler-Euler bubble transport | ||
| !! @param idir Direction index | ||
| !! @param q_prim_vf Primitive variables |
There was a problem hiding this comment.
Missing @param divu_in in s_compute_bubbles_EE_rhs docstring.
The new doc block documents idir and q_prim_vf but omits the third parameter divu_in. The inline !< comment on line 102 won't appear in the generated parameter table; a @param entry is needed for a complete Doxygen listing.
📝 Proposed fix
!> Compute the right-hand side for Euler-Euler bubble transport
!! `@param` idir Direction index
!! `@param` q_prim_vf Primitive variables
+ !! `@param` divu_in Matrix for div(u), updated in-place
subroutine s_compute_bubbles_EE_rhs(idir, q_prim_vf, divu_in)Based on learnings: "Add Doxygen docstrings in source files for new public routines."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| !> Compute the right-hand side for Euler-Euler bubble transport | |
| !! @param idir Direction index | |
| !! @param q_prim_vf Primitive variables | |
| !> Compute the right-hand side for Euler-Euler bubble transport | |
| !! `@param` idir Direction index | |
| !! `@param` q_prim_vf Primitive variables | |
| !! `@param` divu_in Matrix for div(u), updated in-place | |
| subroutine s_compute_bubbles_EE_rhs(idir, q_prim_vf, divu_in) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/simulation/m_bubbles_EE.fpp` around lines 95 - 97, Add a `@param` entry for
divu_in to the Doxygen docblock for s_compute_bubbles_EE_rhs so the third
parameter is documented (the inline `!<` comment at line 102 is not picked up by
Doxygen); update the comment block that currently lists `@param` idir and `@param`
q_prim_vf to include `@param divu_in` with a short description of what divu_in
represents (e.g., divergence of velocity) so the generated parameter table is
complete.
| !! that are needed for the bubble modeling | ||
| !! @param q_prim_vf Primitive variables | ||
| !! @param q_cons_vf Conservative variables | ||
| !! @param rhs_vf Right-hand side variables |
There was a problem hiding this comment.
Missing @param divu_in in s_compute_bubble_EE_source docstring.
Adding @param rhs_vf was correct, but divu_in (the fourth parameter) still has no @param entry in the block header — only an inline !< on line 167 that Doxygen won't promote to the parameter table.
📝 Proposed fix
!! `@param` q_cons_vf Conservative variables
!! `@param` rhs_vf Right-hand side variables
+ !! `@param` divu_in Matrix for div(u)
impure subroutine s_compute_bubble_EE_source(q_cons_vf, q_prim_vf, rhs_vf, divu_in)Based on learnings: "Add Doxygen docstrings in source files for new public routines."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| !! @param rhs_vf Right-hand side variables | |
| !! `@param` q_cons_vf Conservative variables | |
| !! `@param` rhs_vf Right-hand side variables | |
| !! `@param` divu_in Matrix for div(u) | |
| impure subroutine s_compute_bubble_EE_source(q_cons_vf, q_prim_vf, rhs_vf, divu_in) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/simulation/m_bubbles_EE.fpp` at line 162, The function
s_compute_bubble_EE_source has a missing `@param` entry for the fourth parameter
divu_in in its Doxygen block (you added `@param` rhs_vf but left divu_in only as
an inline !<), so update the docstring for s_compute_bubble_EE_source to include
a proper "@param divu_in" line in the block header with a short description of
what divu_in represents/contains; ensure this matches the existing style used
for "@param rhs_vf" so Doxygen will include it in the generated parameter table.
| !! @param btensor_in Left Cauchy-Green deformation tensor | ||
| !! @param q_prim_vf Primitive variables | ||
| !! @param btensor is the output | ||
| !! @param G_param Elastic shear modulus | ||
| !! @param j x-direction cell index | ||
| !! @param k y-direction cell index | ||
| !! @param l z-direction cell index |
There was a problem hiding this comment.
@param btensor_in should use [in,out] direction qualifier — it is mutated inside both solvers
In both s_neoHookean_cauchy_solver (lines 244–246) and s_Mooney_Rivlin_cauchy_solver (lines 288–290), btensor_in is declared intent(inout) and its diagonal entries are overwritten in-place with the deviatoric values before being used for the stress update. Documenting it as a plain @param (input-only implication) misrepresents the contract for both callers and API readers.
Similarly, q_prim_vf in the same blocks is intent(inout) and has stress/invariant values written into it — the existing @param q_prim_vf Primitive variables line (unchanged) carries the same omission.
📝 Suggested direction qualifiers for both solvers
- !! `@param` btensor_in Left Cauchy-Green deformation tensor
+ !! `@param`[in,out] btensor_in Left Cauchy-Green deformation tensor (diagonal entries replaced with deviatoric in-place)
- !! `@param` q_prim_vf Primitive variables
+ !! `@param`[in,out] q_prim_vf Primitive variables (stress and elastic invariant entries updated)
- !! `@param` G_param Elastic shear modulus
+ !! `@param`[in] G_param Elastic shear modulus
- !! `@param` j x-direction cell index
- !! `@param` k y-direction cell index
- !! `@param` l z-direction cell index
+ !! `@param`[in] j x-direction cell index
+ !! `@param`[in] k y-direction cell index
+ !! `@param`[in] l z-direction cell index(Apply the same change to both s_neoHookean_cauchy_solver and s_Mooney_Rivlin_cauchy_solver.)
Also applies to: 262-267
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/simulation/m_hyperelastic.fpp` around lines 219 - 224, Update the
parameter documentation to reflect mutation: change the `@param` entries for
btensor_in and q_prim_vf to include the direction qualifier [in,out] in the
docblocks for both s_neoHookean_cauchy_solver and s_Mooney_Rivlin_cauchy_solver
(and the duplicate docblock at lines referenced around 262–267), since
btensor_in and q_prim_vf are declared intent(inout) and are modified in-place
(diagonal overwritten and stress/invariants written); ensure the two `@param`
lines read e.g. "@param btensor_in [in,out] Left Cauchy-Green deformation
tensor" and "@param q_prim_vf [in,out] Primitive variables" so callers and
readers see the correct contract.
- gen_api_landing.py: scan .f90 files (not just .fpp), add encoding and mkdir, add future annotations - fix_file_briefs.py: exclude module procedure from regex, add encoding, add future annotations - CMakeLists.txt: add source file DEPENDS to stamp commands so they re-run when Fortran sources change Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add ~170 !> @brief one-line descriptions to previously undocumented subroutines/functions across all 45 source files - Add cross-navigation links (User Guide, Pre-Process/Simulation/Post- Process API) injected at the top of the Doxygen sidebar via JavaScript - Fix stale @param ib in s_ib_3D_airfoil Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…int for missing briefs - Rewrite ~51 module-level !> @brief descriptions to be concise and descriptive (one sentence, no boilerplate) - Update gen_api_landing.py to extract briefs from source files and render module tables with descriptions on API landing pages - Add check_module_briefs() linter to lint_docs.py ensuring every m_*.fpp/.f90 has a module-level !> @brief before the module declaration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 57 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="toolchain/mfc/lint_docs.py">
<violation number="1" location="toolchain/mfc/lint_docs.py:817">
P2: The new module brief check treats the common "Contains module ..." @brief as invalid, so existing modules like m_data_input will be flagged as missing briefs. This creates false lint failures and conflicts with the current auto-fixed @brief format.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1167 +/- ##
==========================================
- Coverage 44.06% 44.05% -0.02%
==========================================
Files 70 70
Lines 20431 20498 +67
Branches 1975 1990 +15
==========================================
+ Hits 9003 9030 +27
- Misses 10291 10329 +38
- Partials 1137 1139 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
User description
Summary
docs/{target}/readme.md) at build time viagen_api_landing.py, which scanssrc/{target}/*.fppfor module names and produces@reflinks with correct Fortran case handlingSOURCE_BROWSER = YESin Doxygen so users can click through to annotated source code@filedocumentation blocks to 16 source files; fix 3 malformed Doxygen comment openers (!!>and bare!!)@reffor mixed-case modules (m_bubbles_EE/EL/EL_kernels), correct stale names (m_patches->m_ib_patches)fix_file_briefs.pybuild-time script that auto-fixes@file@briefheaders to match actualmodule/programdeclarations — catches renames, case issues, and missing headers automaticallystrawberryperl.comfrom lychee (frequently times out)Test plan
./mfc.sh build -j 10 -t documentationsucceedsm_bubbles_EE/EL/EL_kernels)index.htmlpagesfix_file_briefs.pyis idempotent (0 changes on second run)./mfc.sh precheckpasses (5/5)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores
CodeAnt-AI Description
Improve API documentation discoverability and source browsing
What Changed
Impact
✅ Clearer API navigation from the docs landing page✅ Clickable, annotated source from module pages✅ Complete and correctly linked module/file lists in API pages💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.