Skip to content

docs: add mdBook documentation site with FG branding - #243

Merged
nh13 merged 6 commits into
mainfrom
docs/nh/mdbook-documentation-site
Apr 7, 2026
Merged

nh13 merged 6 commits into
mainfrom
docs/nh/mdbook-documentation-site

Conversation

@nh13

@nh13 nh13 commented Apr 7, 2026

Copy link
Copy Markdown
Member

Summary

  • New mdBook documentation site with Fulcrum Genomics branding and auto-generated tool/metric reference pages
  • Custom theme: IBM Plex fonts, FG brand colors, Night Blue→Forest Green sidebar gradient, FG logo
  • Sidebar: logo links home, product name in brand colors (fg=blue, umi=green), tagline, corporate link
  • Breadcrumb navigation injected on all guide/tool/metric pages
  • Footer with Visit Us section and quick links (GitHub, API Docs, Issues, Discussions) on all non-index pages
  • Reorganized sidebar: User Guide grouped by topic, Tool Reference in pipeline order, Metrics by type
  • Sidebar collapses by default (fold level=0); localStorage reset on version bump
  • Auto-generated tool reference from clap introspection with descriptions and default values
  • Auto-generated metric reference parsed from Rust source doc comments
  • Updated guides reflecting changes since v0.1.2 (merge, simplex-metrics, new CLI flags)

Test plan

  • cargo docs-build completes without errors
  • Sidebar collapses by default on fresh page load
  • FG logo appears in sidebar and links to home page
  • Breadcrumbs appear on guide/tool/metric pages, not on index
  • Footer with quick links appears on all non-index pages
  • Visit Us section appears at top of index page
  • Inline code is readable (light blue background, dark text)
  • Tool reference index shows descriptions for all commands
  • Menu bar title shows fg in blue, umi in green
  • Print layout hides sidebar and navigation

@nh13 nh13 added the documentation Improvements or additions to documentation label Apr 7, 2026
Adds book.toml configuration, custom CSS theme, and .gitignore entries
for auto-generated documentation content (SUMMARY.md, tools/, metrics/).
@nh13
nh13 force-pushed the docs/nh/mdbook-documentation-site branch from 0bd2872 to da61a6d Compare April 7, 2026 01:15
@nh13
nh13 temporarily deployed to github-actions April 7, 2026 01:15 — with GitHub Actions Inactive
@codecov

codecov Bot commented Apr 7, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.34641% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.97%. Comparing base (cbc40e9) to head (d64420c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/main.rs 95.65% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #243      +/-   ##
==========================================
- Coverage   88.97%   88.97%   -0.01%     
==========================================
  Files         114      114              
  Lines       55481    55488       +7     
==========================================
+ Hits        49366    49372       +6     
- Misses       6115     6116       +1     

☔ 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.

@nh13
nh13 marked this pull request as ready for review April 7, 2026 02:36
@nh13

nh13 commented Apr 7, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Apr 7, 2026 •

Copy link
Copy Markdown

Warning

Rate limit exceeded

@nh13 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 8 minutes and 16 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 8 minutes and 16 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0cd50549-fad3-4194-9219-1408890a774e

📥 Commits

Reviewing files that changed from the base of the PR and between 29caa55 and d64420c.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (18)
  • .readthedocs.yaml
  • crates/xtask/src/generate_summary.rs
  • crates/xtask/src/generate_tools.rs
  • docs/LAST_SYNCED
  • docs/book.toml
  • docs/src/guide/best-practices.md
  • docs/src/guide/consensus-calling.md
  • docs/src/guide/duplex-consensus-calling.md
  • docs/src/guide/getting-started.md
  • docs/src/guide/migration-from-fgbio.md
  • docs/src/guide/performance-tuning.md
  • docs/src/guide/read-structures.md
  • docs/src/guide/tracking-reads.md
  • docs/src/guide/umi-grouping.md
  • docs/src/guide/working-with-metrics.md
  • docs/src/index.md
  • docs/theme/custom.css
  • docs/theme/sidebar-brand.js
📝 Walkthrough

Walkthrough

Adds a new workspace crate crates/xtask with a CLI (xtask) and commands to generate docs: per-tool pages (via clap), metric pages (via syn), and an mdBook SUMMARY.md. Adds mdBook configuration, theme assets (CSS/JS), many guide pages, docs/book.toml, .readthedocs.yaml, docs/LAST_SYNCED, and .gitignore entries. Adds Cargo aliases docs/docs-build. Exposes commands and version modules publicly and refactors many modules to use crate::... paths instead of fgumi_lib::...; widens Extract visibility.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed Title accurately describes the main change: adding a new mdBook documentation site with FG branding and auto-generated reference pages.
Description check ✅ Passed Description clearly details the documentation site features, custom branding, auto-generation approach, and test plan. Well-organized and relates directly to the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/nh/mdbook-documentation-site

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai

coderabbitai Bot commented Apr 7, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13

nh13 commented Apr 7, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Apr 7, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 14

🧹 Nitpick comments (4)
docs/src/guide/consensus-calling.md (1)

29-31: Consider adding language hints to fenced code blocks.

markdownlint flags these formula blocks as missing language specifiers. Using text or leaving empty is fine for formulas, but explicit text silences the warning.

Example fix for one block
-```
+```text
 Q' = min(Q - S_Q, M_Q)
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @docs/src/guide/consensus-calling.md around lines 29 - 31, The fenced formula
block containing "Q' = min(Q - S_Q, M_Q)" is missing a language specifier which
triggers markdownlint; update the fenced code block in
docs/src/guide/consensus-calling.md to add a language hint (e.g. use text or an empty specifier) so the linter is satisfied. Locate the block showing the formula "Q' = min(Q - S_Q, M_Q)" and change the opening backticks to include the language hint (for example, replace with ```text) for consistent handling of
formula blocks.


</details>

</blockquote></details>
<details>
<summary>docs/src/guide/duplex-consensus-calling.md (1)</summary><blockquote>

`44-48`: **Consider adding a language hint to the code block.**

Similar to other formula/example blocks, using `text` silences MD040.



<details>
<summary>Example</summary>

```diff
-```
+```text
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @docs/src/guide/duplex-consensus-calling.md around lines 44 - 48, Update the
fenced code block that contains the three nucleotide sequence lines (lines
starting with "1: ACGTGACTGACTAGCTTTTTTT-AGACTAGCTACTACT", etc.) by adding the
language hint "text" to the opening fence (change totext) so the block
is treated as plain text and MD040 is silenced; leave the block contents
unchanged.


</details>

</blockquote></details>
<details>
<summary>.readthedocs.yaml (1)</summary><blockquote>

`6-12`: **Pin the RTD Rust toolchain to match your workspace requirement.**

RTD currently uses `rust: "latest"`, which creates non-reproducible builds. Your workspace declares `rust-version = "1.87.0"` in Cargo.toml—pin RTD to this same version for consistency.

<details>
<summary>Suggested change</summary>

```diff
 build:
   os: ubuntu-24.04
   tools:
-    rust: "latest"
+    rust: "1.87"
   commands:
     # Install mdbook
-    - cargo install mdbook --version 0.5.2
+    - cargo install mdbook --locked --version 0.5.2
```
</details>

Adding `--locked` to the mdbook install also ensures reproducible dependency resolution.

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In @.readthedocs.yaml around lines 6 - 12, The RTD config uses rust: "latest"
and installs mdbook without locking, which makes builds non-reproducible; update
the rust key to match the workspace rust-version "1.87.0" and add --locked to
the mdbook install command (the cargo install mdbook --version 0.5.2 line) so
dependency resolution is reproducible, leaving the cargo run --package xtask
--release -- generate-docs step unchanged.
```

</details>

</blockquote></details>
<details>
<summary>docs/src/guide/getting-started.md (1)</summary><blockquote>

`173-178`: **Clarify `--min-reads` parameter format**

The example shows `--min-reads 1,1,1` (duplex format with 3 values) without explaining that simplex workflows would use a single value like `--min-reads 1`. Since the guide presents multiple consensus calling options (simplex/duplex/codec), readers may be unclear which parameter format to use.


<details>
<summary>Suggested clarification</summary>

```diff
 ### 7. Filter Consensus Reads
 
 Filter consensus reads based on quality metrics:
 
+**For simplex consensus:**
+```bash
+fgumi filter \
+  --input consensus.bam \
+  --output filtered.bam \
+  --ref ref.fa \
+  --min-reads 1
+```
+
+**For duplex consensus:**
 ```bash
 fgumi filter \
-  --input consensus.bam \
+  --input duplex.bam \
   --output filtered.bam \
   --ref ref.fa \
-  --min-reads 1,1,1
+  --min-reads 1,1,1  # duplex, AB, BA thresholds
 ```
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@docs/src/guide/getting-started.md` around lines 173 - 178, Clarify the
--min-reads parameter format in the fgumi filter example: explain that simplex
workflows accept a single integer (e.g., --min-reads 1) while duplex workflows
require three comma-separated values (e.g., --min-reads 1,1,1) corresponding to
AB/BA/consensus thresholds; update the examples around the fgumi filter
invocation (references: command "fgumi filter", parameter "--min-reads", input
filenames "consensus.bam" vs "duplex.bam") to show both the simplex and duplex
forms and a short inline comment indicating the meaning of the three values.
```

</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @crates/xtask/src/generate_metrics.rs:

  • Around line 153-159: The Type::Path branch in type_to_string strips generic
    arguments by only joining segment idents; update type_to_string to render each
    path segment with its PathArguments (handle syn::PathArguments::AngleBracketed
    by converting each generic argument back to a string recursively, and preserve
    other argument variants), e.g. iterate p.path.segments and for each segment
    append its ident plus any angle-bracketed arguments formatted (reusing
    type_to_string for nested syn::Type arguments) so Option and HashMap<K, V>
    keep their generics; keep the current fallback (quote::quote!(#ty).to_string())
    for unsupported cases.
  • Around line 196-199: The index currently writes links as "metrics/{slug}.md"
    which becomes "metrics/metrics/{slug}.md" because README.md is generated into
    docs/src/metrics/; update the link target in the loop that writes the table (the
    block iterating over metrics, using variables metric, slug, md and the call to
    writeln!) to point to the local metric file (e.g., "{slug}.md" or "./{slug}.md")
    instead of "metrics/{slug}.md" so the links resolve correctly from the metrics
    directory.

In @crates/xtask/src/generate_tools.rs:

  • Around line 64-89: The parsers registry in the parsers Vec is missing tools
    referenced elsewhere (simplex-metrics, merge, compare), so update the parsers
    list (the Vec assigned to parsers and the imports at top of the file) to include
    entries for "simplex-metrics", "merge", and "compare" mapping to their
    CommandFactory::command functions (e.g. add ("simplex-metrics",
    <simplex_metrics::SimplexMetrics as CommandFactory>::command), ("merge",
    <merge::Merge as CommandFactory>::command), ("compare", <compare::Compare as
    CommandFactory>::command) or the actual module/type names used in the codebase);
    ensure you also add corresponding use imports for the modules (simplex_metrics,
    merge, compare) so render_tool_page(), collect_commands(), and sidebar
    generation will find those tools.
  • Around line 295-300: The generated README links use root-relative paths (e.g.
    "tools/...") which break when README.md lives in docs/src/tools/; in the loop
    over by_category (the for (category, tools) in by_category block) change how the
    link target is built: compute a display_path by removing the leading "tools/"
    prefix from the path (e.g. with strip_prefix or trim_start_matches) and use that
    display_path when writing the table row (the writeln!(md, "| {name}
    | {description} |") call), leaving the original path unchanged for SUMMARY.md
    generation elsewhere.

In @crates/xtask/src/main.rs:

  • Around line 1-7: Add the crate-level deny lint by inserting the crate
    attribute #![deny(unsafe_code)] at the top of the crate root (main.rs) before
    any use or mod declarations so the entire xtask crate enforces no unsafe code;
    update the file containing the use declarations and mod statements (main.rs) to
    include that attribute as the very first line.

In @docs/src/guide/best-practices.md:

  • Around line 399-409: The documentation's Phase 2b title "Grouped BAM →
    Filtered Consensus" conflicts with the example which reruns fgumi group on
    aligned.bam; fix by either renaming the phase to reflect starting from raw
    aligned.bam (e.g., "Aligned BAM → Filtered Consensus") or change the example to
    start from a grouped file (e.g., use a grouped.bam input instead of aligned.bam
    and remove the fgumi group call), ensuring the header and the fgumi commands
    (fgumi group / fgumi simplex) and filenames (aligned.bam, grouped.bam)
    consistently match.
  • Around line 380-383: The documentation currently explains --min-reads as
    "reads" but the flag counts templates/molecules, not individual read records;
    update the wording where --min-reads is described (the lines showing "For
    duplex, --min-reads 10,5,3 means:") to state that the thresholds are per input
    template/molecule and that on paired-end data a read pair counts as a single
    template (e.g., "10 templates for final duplex consensus, 5 templates for AB
    single-strand consensus, 3 templates for BA single-strand consensus; paired-end
    read pairs count as one template, not two reads").

In @docs/src/guide/duplex-consensus-calling.md:

  • Around line 30-34: Hyphenate the compound adjective "low quality" wherever it
    modifies "bases": change the phrase in the sentence "Reads can be end-trimmed to
    remove low quality bases." to "low-quality bases", and update the heading
    "Masking Low Quality Bases" to "Masking Low-Quality Bases" so the adjective is
    correctly hyphenated.

In @docs/src/guide/performance-tuning.md:

  • Around line 27-35: The doc incorrectly equates --threads 1 with omitting the
    --threads flag; update the "Single-threaded Mode" and usage lines to clearly
    distinguish the two modes: state that omitting the flag uses the optimized fast
    path (no-flag fast path) while explicitly passing --threads 1 uses the
    single-threaded pipeline (not the fast path), keep the existing --threads N
    description for multi-threaded mode, and adjust the "Behavior" and "Best for"
    bullets to reflect the behavioral and performance differences between no-flag
    fast path and explicit --threads 1.

In @docs/src/guide/read-structures.md:

  • Around line 60-71: The markdown code fence containing the grammar (starting
    with and listing rules like ,
    , , etc.) is missing a language tag and triggers
    MD040; update the opening triple-backtick to include a language identifier
    (e.g., ```text) so the block becomes a fenced code block with a specified
    language. Ensure you only modify the opening fence and keep the grammar content
    unchanged.

In @docs/src/guide/tracking-reads.md:

  • Around line 13-18: The fenced code block containing the arrow diagram (the
    three-line block starting with "x: R1----------------->") lacks a language label
    and triggers MD040; update that fenced block by adding an explicit language tag
    (e.g., ```text) immediately after the opening backticks so the block becomes

In `@docs/src/guide/working-with-metrics.md`:
- Around line 62-65: Update the wording for the two table entries
`PREFIX.family_sizes.txt` and `PREFIX.grouping_metrics.txt` to use
template-based language: clarify that UMI family sizes and the `--min-reads`
threshold count templates (molecules), not individual SAM records, and add a
short sentence explicitly stating that a paired-end read pair (R1+R2) equals a
single template so a one-fragment family is counted as size 1; make the
identical wording change at the other occurrence noted (lines 108-109) so both
locations consistently describe `--min-reads` as template-level semantics.

In `@docs/theme/custom.css`:
- Line 7: The `@import` in custom.css uses the url(...) form which violates
Stylelint's import-notation rule; update the `@import` statement in
docs/theme/custom.css to use the plain string form (remove url(...) and keep the
same quoted font URL) so it reads as an `@import` with a string literal, ensuring
the import-notation lint rule passes and the fonts remain unchanged.

In `@docs/theme/sidebar-brand.js`:
- Around line 71-72: Hard-coded root "/" hrefs (e.g., the header.innerHTML using
LOGO_SVG) and the p.endsWith('/') check in injectFooter() assume site root
hosting and break on subpath/pretty-URL deployments; compute the docs root/base
path at runtime (derive from a canonical base like themeConfig.baseUrl or from
window.location.pathname/URL) and use that base when building links (replace "/"
with the computed docsRoot) and when testing pages (replace p.endsWith('/') with
a check that strips or compares against the docsRoot or uses URL.pathname
semantics); update all usages (header.innerHTML with LOGO_SVG, breadcrumb/link
construction at the other occurrences, and injectFooter()) to use this computed
docs root.

---

Nitpick comments:
In @.readthedocs.yaml:
- Around line 6-12: The RTD config uses rust: "latest" and installs mdbook
without locking, which makes builds non-reproducible; update the rust key to
match the workspace rust-version "1.87.0" and add --locked to the mdbook install
command (the cargo install mdbook --version 0.5.2 line) so dependency resolution
is reproducible, leaving the cargo run --package xtask --release --
generate-docs step unchanged.

In `@docs/src/guide/consensus-calling.md`:
- Around line 29-31: The fenced formula block containing "Q' = min(Q - S_Q,
M_Q)" is missing a language specifier which triggers markdownlint; update the
fenced code block in docs/src/guide/consensus-calling.md to add a language hint
(e.g. use ```text or an empty specifier) so the linter is satisfied. Locate the
block showing the formula "Q' = min(Q - S_Q, M_Q)" and change the opening
backticks to include the language hint (for example, replace ``` with ```text)
for consistent handling of formula blocks.

In `@docs/src/guide/duplex-consensus-calling.md`:
- Around line 44-48: Update the fenced code block that contains the three
nucleotide sequence lines (lines starting with "1:
ACGTGACTGACTAGCTTTTTTT-AGACTAGCTACTACT", etc.) by adding the language hint
"text" to the opening fence (change ``` to ```text) so the block is treated as
plain text and MD040 is silenced; leave the block contents unchanged.

In `@docs/src/guide/getting-started.md`:
- Around line 173-178: Clarify the --min-reads parameter format in the fgumi
filter example: explain that simplex workflows accept a single integer (e.g.,
--min-reads 1) while duplex workflows require three comma-separated values
(e.g., --min-reads 1,1,1) corresponding to AB/BA/consensus thresholds; update
the examples around the fgumi filter invocation (references: command "fgumi
filter", parameter "--min-reads", input filenames "consensus.bam" vs
"duplex.bam") to show both the simplex and duplex forms and a short inline
comment indicating the meaning of the three values.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 07e1c33d-1256-42c6-b3b8-567d08b25a02

📥 Commits

Reviewing files that changed from the base of the PR and between cbc40e9 and da61a6d.

⛔ Files ignored due to path filters (3)
  • Cargo.lock is excluded by !**/*.lock
  • docs/src/images/fgumi_subway.png is excluded by !**/*.png
  • docs/src/images/fgumi_subway.svg is excluded by !**/*.svg
📒 Files selected for processing (62)
  • .cargo/config.toml
  • .gitignore
  • .readthedocs.yaml
  • Cargo.toml
  • crates/xtask/Cargo.toml
  • crates/xtask/src/generate_docs.rs
  • crates/xtask/src/generate_metrics.rs
  • crates/xtask/src/generate_summary.rs
  • crates/xtask/src/generate_tools.rs
  • crates/xtask/src/main.rs
  • docs/LAST_SYNCED
  • docs/book.toml
  • docs/src/guide/best-practices.md
  • docs/src/guide/consensus-calling.md
  • docs/src/guide/duplex-consensus-calling.md
  • docs/src/guide/getting-started.md
  • docs/src/guide/migration-from-fgbio.md
  • docs/src/guide/performance-tuning.md
  • docs/src/guide/read-structures.md
  • docs/src/guide/tracking-reads.md
  • docs/src/guide/umi-grouping.md
  • docs/src/guide/working-with-metrics.md
  • docs/src/index.md
  • docs/theme/custom.css
  • docs/theme/sidebar-brand.js
  • src/lib/commands/clip.rs
  • src/lib/commands/codec.rs
  • src/lib/commands/command.rs
  • src/lib/commands/common.rs
  • src/lib/commands/compare/bams.rs
  • src/lib/commands/compare/metrics.rs
  • src/lib/commands/compare/mod.rs
  • src/lib/commands/compare/raw_compare.rs
  • src/lib/commands/consensus_runner.rs
  • src/lib/commands/correct.rs
  • src/lib/commands/dedup.rs
  • src/lib/commands/downsample.rs
  • src/lib/commands/duplex.rs
  • src/lib/commands/duplex_metrics.rs
  • src/lib/commands/extract.rs
  • src/lib/commands/fastq.rs
  • src/lib/commands/filter.rs
  • src/lib/commands/group.rs
  • src/lib/commands/merge.rs
  • src/lib/commands/mod.rs
  • src/lib/commands/review.rs
  • src/lib/commands/shared_metrics.rs
  • src/lib/commands/simplex.rs
  • src/lib/commands/simplex_metrics.rs
  • src/lib/commands/simulate/common.rs
  • src/lib/commands/simulate/consensus_reads.rs
  • src/lib/commands/simulate/correct_reads.rs
  • src/lib/commands/simulate/fastq_reads.rs
  • src/lib/commands/simulate/grouped_reads.rs
  • src/lib/commands/simulate/mapped_reads.rs
  • src/lib/commands/simulate/mod.rs
  • src/lib/commands/simulate/sort.rs
  • src/lib/commands/sort.rs
  • src/lib/commands/zipper.rs
  • src/lib/mod.rs
  • src/lib/version.rs
  • src/main.rs

Comment thread crates/xtask/src/generate_metrics.rs
Comment thread crates/xtask/src/generate_metrics.rs Outdated
Comment thread crates/xtask/src/generate_tools.rs
Comment thread crates/xtask/src/generate_tools.rs Outdated
Comment thread crates/xtask/src/main.rs
Comment thread docs/src/guide/read-structures.md Outdated
Comment thread docs/src/guide/tracking-reads.md Outdated
Comment thread docs/src/guide/working-with-metrics.md
Comment thread docs/theme/custom.css Outdated
Comment thread docs/theme/sidebar-brand.js Outdated
@nh13
nh13 force-pushed the docs/nh/mdbook-documentation-site branch from da61a6d to 6110625 Compare April 7, 2026 05:43
@nh13
nh13 temporarily deployed to github-actions April 7, 2026 05:43 — with GitHub Actions Inactive
nh13 added 2 commits April 6, 2026 23:10
Moves the commands module and version module from the binary crate into
the library crate (fgumi_lib) so they can be imported by other workspace
crates. Replaces enum_dispatch with a manual match for cross-crate
command dispatch. Fixes include_str paths and visibility for the moved
structs.
@nh13
nh13 force-pushed the docs/nh/mdbook-documentation-site branch from 6110625 to 29caa55 Compare April 7, 2026 06:32
@nh13
nh13 temporarily deployed to github-actions April 7, 2026 06:33 — with GitHub Actions Inactive

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/lib/commands/sort.rs (1)

899-916: ⚠️ Potential issue | 🟡 Minor

This test still flakes on low-memory runners.

With 4 threads, both reserve sizes collapse to the same MIN_MEMORY_PER_THREAD * 4 budget when total RAM is <= 1152 MiB, so the strict < assertion is wrong in that environment.

Diff
     let small_reserve = resolve_memory_limit(
         MemoryLimit::Auto,
         MemoryReserve::Fixed(128 * 1024 * 1024),
         4,
         true,
     )
     .expect("should succeed");
-    assert!(large_reserve < small_reserve);
+
+    let mut system = sysinfo::System::new();
+    system.refresh_memory();
+    let total = usize::try_from(system.total_memory()).unwrap_or(usize::MAX);
+    let floor_threshold = (128 * 1024 * 1024) + (MIN_MEMORY_PER_THREAD * 4);
+
+    if total <= floor_threshold {
+        assert_eq!(large_reserve, small_reserve);
+    } else {
+        assert!(large_reserve < small_reserve);
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/commands/sort.rs` around lines 899 - 916, The test assumes
large_reserve < small_reserve but on low-RAM CI both collapse to the same
per-thread floor; update the assertion in the test around resolve_memory_limit
(the block creating large_reserve and small_reserve using MemoryLimit::Auto and
MemoryReserve::Fixed) to allow equality (e.g., use <= or assert!(large_reserve
== small_reserve || large_reserve < small_reserve)) and add a short comment
mentioning the low-memory collapse case so the intent is clear.
🧹 Nitpick comments (1)
src/main.rs (1)

13-13: Import placed after constant definition.

use env_logger::Env; is positioned after the STYLES constant. Consider moving it to the import block at lines 3-5 for consistency.

Suggested diff
 use anyhow::Result;
 use clap::Parser;
 use clap::builder::styling::{AnsiColor, Effects, Styles};
+use env_logger::Env;

 /// Custom styles for CLI help output
 const STYLES: Styles = Styles::styled()
     .header(AnsiColor::Green.on_default().effects(Effects::BOLD))
     .usage(AnsiColor::Green.on_default().effects(Effects::BOLD))
     .literal(AnsiColor::Cyan.on_default().effects(Effects::BOLD))
     .placeholder(AnsiColor::Cyan.on_default());
-use env_logger::Env;
 use fgumi_lib::commands::clip::Clip;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main.rs` at line 13, Move the stray use statement into the module import
block: relocate the `use env_logger::Env;` line so it sits with the other
imports at the top of the file (near the existing use declarations) instead of
after the `STYLES` constant; this keeps all imports grouped together and
preserves the `STYLES` constant definition locality.
🤖 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/src/guide/read-structures.md`:
- Around line 61-63: The current grammar allows <read-structure> to be empty
because both <fixed-structure> and <variable-segment> permit ""; change the
rules so at least one segment is required by removing the empty alternatives and
making <read-structure> start with a required segment (<fixed-length> or
<variable-length>) followed by zero-or-more ("<operator>
<fixed-length|variable-length>") repeats. Update the definitions for
<read-structure>, <fixed-structure>, and <variable-segment> accordingly so
neither <fixed-structure> nor <variable-segment> can be empty and the parse
always includes >=1 segment.

---

Outside diff comments:
In `@src/lib/commands/sort.rs`:
- Around line 899-916: The test assumes large_reserve < small_reserve but on
low-RAM CI both collapse to the same per-thread floor; update the assertion in
the test around resolve_memory_limit (the block creating large_reserve and
small_reserve using MemoryLimit::Auto and MemoryReserve::Fixed) to allow
equality (e.g., use <= or assert!(large_reserve == small_reserve ||
large_reserve < small_reserve)) and add a short comment mentioning the
low-memory collapse case so the intent is clear.

---

Nitpick comments:
In `@src/main.rs`:
- Line 13: Move the stray use statement into the module import block: relocate
the `use env_logger::Env;` line so it sits with the other imports at the top of
the file (near the existing use declarations) instead of after the `STYLES`
constant; this keeps all imports grouped together and preserves the `STYLES`
constant definition locality.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 98db6bdc-23a4-481e-955d-868529125280

📥 Commits

Reviewing files that changed from the base of the PR and between 6110625 and 29caa55.

⛔ Files ignored due to path filters (3)
  • Cargo.lock is excluded by !**/*.lock
  • docs/src/images/fgumi_subway.png is excluded by !**/*.png
  • docs/src/images/fgumi_subway.svg is excluded by !**/*.svg
📒 Files selected for processing (65)
  • .cargo/config.toml
  • .readthedocs.yaml
  • Cargo.toml
  • crates/xtask/Cargo.toml
  • crates/xtask/src/generate_docs.rs
  • crates/xtask/src/generate_metrics.rs
  • crates/xtask/src/generate_summary.rs
  • crates/xtask/src/generate_tools.rs
  • crates/xtask/src/main.rs
  • docs/LAST_SYNCED
  • docs/book.toml
  • docs/src/guide/best-practices.md
  • docs/src/guide/consensus-calling.md
  • docs/src/guide/duplex-consensus-calling.md
  • docs/src/guide/getting-started.md
  • docs/src/guide/migration-from-fgbio.md
  • docs/src/guide/performance-tuning.md
  • docs/src/guide/read-structures.md
  • docs/src/guide/tracking-reads.md
  • docs/src/guide/umi-grouping.md
  • docs/src/guide/working-with-metrics.md
  • docs/src/index.md
  • docs/theme/custom.css
  • docs/theme/sidebar-brand.js
  • src/lib/commands/clip.rs
  • src/lib/commands/codec.rs
  • src/lib/commands/command.rs
  • src/lib/commands/common.rs
  • src/lib/commands/compare/bams.rs
  • src/lib/commands/compare/metrics.rs
  • src/lib/commands/compare/mod.rs
  • src/lib/commands/compare/raw_compare.rs
  • src/lib/commands/consensus_runner.rs
  • src/lib/commands/correct.rs
  • src/lib/commands/dedup.rs
  • src/lib/commands/downsample.rs
  • src/lib/commands/duplex.rs
  • src/lib/commands/duplex_metrics.rs
  • src/lib/commands/extract.rs
  • src/lib/commands/fastq.rs
  • src/lib/commands/filter.rs
  • src/lib/commands/group.rs
  • src/lib/commands/merge.rs
  • src/lib/commands/mod.rs
  • src/lib/commands/review.rs
  • src/lib/commands/shared_metrics.rs
  • src/lib/commands/simplex.rs
  • src/lib/commands/simplex_metrics.rs
  • src/lib/commands/simulate/common.rs
  • src/lib/commands/simulate/consensus_reads.rs
  • src/lib/commands/simulate/correct_reads.rs
  • src/lib/commands/simulate/fastq_reads.rs
  • src/lib/commands/simulate/grouped_reads.rs
  • src/lib/commands/simulate/mapped_reads.rs
  • src/lib/commands/simulate/mod.rs
  • src/lib/commands/simulate/sort.rs
  • src/lib/commands/sort.rs
  • src/lib/commands/zipper.rs
  • src/lib/mod.rs
  • src/lib/sort/inline_buffer.rs
  • src/lib/sort/segmented_buf.rs
  • src/lib/unified_pipeline/rebalancer.rs
  • src/lib/validation.rs
  • src/lib/version.rs
  • src/main.rs
💤 Files with no reviewable changes (1)
  • src/lib/sort/inline_buffer.rs
✅ Files skipped from review due to trivial changes (37)
  • .cargo/config.toml
  • src/lib/commands/merge.rs
  • docs/LAST_SYNCED
  • src/lib/sort/segmented_buf.rs
  • src/lib/commands/fastq.rs
  • src/lib/commands/consensus_runner.rs
  • src/lib/commands/compare/metrics.rs
  • src/lib/commands/simulate/correct_reads.rs
  • .readthedocs.yaml
  • src/lib/commands/compare/bams.rs
  • src/lib/commands/downsample.rs
  • src/lib/validation.rs
  • src/lib/unified_pipeline/rebalancer.rs
  • src/lib/commands/shared_metrics.rs
  • src/lib/commands/simulate/consensus_reads.rs
  • src/lib/commands/review.rs
  • docs/book.toml
  • src/lib/commands/simulate/grouped_reads.rs
  • src/lib/commands/correct.rs
  • src/lib/commands/clip.rs
  • src/lib/commands/duplex.rs
  • src/lib/commands/simplex_metrics.rs
  • crates/xtask/Cargo.toml
  • src/lib/commands/codec.rs
  • src/lib/commands/zipper.rs
  • src/lib/commands/duplex_metrics.rs
  • src/lib/commands/filter.rs
  • docs/src/guide/working-with-metrics.md
  • docs/src/guide/duplex-consensus-calling.md
  • docs/src/guide/migration-from-fgbio.md
  • docs/src/index.md
  • docs/src/guide/tracking-reads.md
  • docs/theme/custom.css
  • docs/theme/sidebar-brand.js
  • docs/src/guide/getting-started.md
  • docs/src/guide/umi-grouping.md
  • docs/src/guide/performance-tuning.md
🚧 Files skipped from review as they are similar to previous changes (14)
  • src/lib/mod.rs
  • Cargo.toml
  • src/lib/commands/simulate/mapped_reads.rs
  • src/lib/commands/extract.rs
  • src/lib/commands/simplex.rs
  • src/lib/commands/dedup.rs
  • src/lib/commands/common.rs
  • src/lib/commands/simulate/fastq_reads.rs
  • crates/xtask/src/main.rs
  • src/lib/commands/simulate/common.rs
  • crates/xtask/src/generate_summary.rs
  • src/lib/commands/group.rs
  • crates/xtask/src/generate_tools.rs
  • crates/xtask/src/generate_metrics.rs

Comment thread docs/src/guide/read-structures.md Outdated
nh13 added 3 commits April 6, 2026 23:50
…ges since v0.1.2

Updates all guide pages to reflect changes made to fgumi since the initial
documentation was written (ec8c1ab..2f14de1):

- getting-started: add merge step, simplex-metrics option, zipper BAM note,
  --metrics for group, --cell-tag for sort with single-cell data
- best-practices: add merge, simplex-metrics (with yield curve explanation),
  --metrics prefix for group, --allow-unmapped, boolean flag values, updated
  dedup/filter examples to use explicit bool values, cell-tag in sort
- umi-grouping: document --allow-unmapped, expand --metrics prefix and
  position_group_sizes.txt, improve cell barcode section, update sort order
  to mention CB tag inclusion
- working-with-metrics: add simplex-metrics to commands table and output files,
  document position_group_sizes.txt, document --metrics prefix for group
- migration-from-fgbio: add merge and simplex-metrics to command mapping,
  document boolean flag values, note --sort-order removal from simplex/codec,
  note group position_group_sizes metric, note sort --cell-tag
- performance-tuning: add merge and metrics commands to command-specific
  section, document zipper raw-byte merge and sort LoserTree improvements
- consensus-calling: note removal of --sort-order from simplex and codec
- docs/LAST_SYNCED: record the commit through which guides were last reviewed
- Add Fulcrum Genomics sidebar logo (inlined SVG), product name in brand
  colors (fg=blue, umi=green), tagline, and corporate link
- Add breadcrumb navigation on guide/tool/metric pages
- Add page footer with Visit Us section and quick links (GitHub, API Docs,
  Issues, Discussions) injected on all non-index pages
- Reorganize sidebar: User Guide grouped into Core Concepts / Consensus /
  Advanced Topics; Tool Reference in pipeline order with GROUP+DEDUP merged;
  Metrics grouped by type
- Set sidebar fold level=0 (all collapsed by default)
- Rename Introduction to Home in sidebar
- Fix inline code contrast (light blue background, dark blue text)
- Fix tool index missing descriptions
- Increase base font size to 18px; use IBM Plex Sans/Mono throughout
- Widen content area (max-width 1000px, up from 750px default)
- Polish: tighter heading line-height, h4 sizing/color, blockquote
  background, table alternate rows, button padding, print styles,
  selective localStorage clearing, menu title coloring
RTD only supports specific pinned Rust versions; 1.87 is not in the
allowed list. Using "latest" is correct since rust-toolchain.toml in
the repo pins the actual toolchain version at build time.
@nh13
nh13 force-pushed the docs/nh/mdbook-documentation-site branch from 29caa55 to d64420c Compare April 7, 2026 06:52
@nh13
nh13 temporarily deployed to github-actions April 7, 2026 06:52 — with GitHub Actions Inactive
@nh13
nh13 merged commit 2a50521 into main Apr 7, 2026
9 checks passed
@nh13
nh13 deleted the docs/nh/mdbook-documentation-site branch April 7, 2026 07:19
@nh13 nh13 mentioned this pull request Apr 6, 2026

This branch was previously deployed

1 inactive deployment
github-actions — d64420c3 Deployed Apr 7, 2026 by nh13 via coverage #964
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant