Review Facilitation PR

[dblodgett-usgs]

Code Review Summary: hydroloom v1.0.0 → v1.2.0

Generated with Claude Code for PR #71. Covers all changes since 8e70a8c (v1.0.0) through the 821ffc63a20a4f0a70289f917cde59a88219c9cb (v1.2.0 dev latest).

Documentation Changes

• New vignettes: accumulate_downstream (three accumulation modes with worked examples), network_navigation (DFS navigation including upmain/downmain)
• Updated vignettes: hydroloom (added make_index_ids() content), advanced_network, flow-table, non-dendritic — terminology consistency (diversion→divergence, flowpath→flowline)
• README.Rmd rewritten; inst/CITATION added
• DESCRIPTION — Version 1.0.0→1.2.0, R ≥4.1.0, added webshot/geos to Suggests
• New man pages for add_measures, get_bridge_flowlines, subset_network, to_flownetwork, hy_capabilities, hy_network_type, is_dendritic; updated pages for all modified functions
• New hydroloom name definitions: divergence_fraction, upmain, downmain added to R/00_hydroloom.R

Package Source Changes

New functions (since v1.1.0)

• accumulate_downstream() — Three routing modes: dendritic (default), divergence-apportioned (divergence_fraction), and total upstream (bridge-based double-count prevention). Rewritten from serial loop to matrix-based iteration. #17
• get_bridge_flowlines() — Identifies bridge (cut-edge) flowlines via iterative Tarjan's algorithm
• subset_network() — Upstream subsetting with diversion chasing for non-dendritic networks. #60
• to_flownetwork() — Converts hy objects to a junction table with id, toid, upmain, downmain
• add_measures() — Linear reference positions along aggregate features

S3 class hierarchy (v1.2.0, #73)

New file R/hy_classes.R (428 lines) defines:

hy                        # base: id + orig_names attribute
├── hy_topo               # edge list: id, toid (unique id)
│   └── hy_leveled        # + topo_sort, levelpath, levelpath_outlet_id
└── hy_node               # bipartite graph: id, fromnode, tonode (unique id)

hy_flownetwork            # standalone junction table (not a subclass of hy)

• hy() updated — gains add_topo parameter; auto-classifies output via classify_hy(); sets attr(x, "dendritic")
• hy_reverse() updated — strips all subclasses, not just "hy"
• to_flownetwork() converted from plain function to S3 generic
• 17 existing functions gained S3 methods (.hy, .hy_topo, .hy_node, .hy_leveled) with guided error messages when the wrong network representation is passed
• Producer functions now stamp output: add_toids() → hy_topo, add_levelpaths() → hy_leveled, make_node_topology() → hy_node, to_flownetwork() → hy_flownetwork
• New exports: hy_network_type(), is_dendritic(), hy_capabilities()
• 4 internal dispatch helpers centralize all method boilerplate (hy_classify_and_redispatch, hy_as_dataframe, hy_node_to_topo, hy_topo_to_node)
• Backward compatible — existing data.frame/hy code works without changes. 41 new S3method() registrations + 3 new exports in NAMESPACE.

Reworked functions (since v1.1.0)

• make_index_ids() — Rewritten with mode parameter ("to", "from", "both"). Subsumes make_fromids() and format_index_ids(), both now deprecated.
• add_levelpaths() — Performance rewrite using data.table. reweight() removed. Non-dendritic input now supported.
• navigate_network_dfs() — Added "upmain" and "downmain" modes
• navigate_hydro_network() — Now navigates diverted→main paths; added long-form mode aliases ("downmain", "upmain", "up", "down")
• index_points_to_lines() — New ids parameter; internal matcher() rewritten as matcher_dt() using data.table; geos::geos_buffer() when available
• add_divergence() — Uses make_index_ids(mode = "from"); sets attr(x, "dendritic") <- FALSE
• check_hy_graph() — Uses make_index_ids(mode = "both")
• utils.R — Helpers consolidated; add_toids_internal() gains divergence-fraction support; new data.table imports

Bug fixes

• sort_network() — Duplicate entries in extended attributes (#52)
• add_toids() — return_dendritic = TRUE was mutating fromnode; now preserved
• make_to_dt() — Dendritic branch failed on tibble input (data.table syntax fix)
• Test tolerance updates for Fedora CRAN compatibility (v1.1.3)

Deprecations

• make_fromids() — removed; use make_index_ids(mode = "from")
• format_index_ids() — still exported with warning
• add_toids(return_dendritic = FALSE) — use to_flownetwork()
• Future: hy_topo will require unique id; non-dendritic duplicated-id edge lists must use hy_flownetwork or hy_node

Testing Changes

• New test files: test_accumulate.R (3 routing modes, simple/complex diversions), test_get_bridges.R (full Tarjan coverage), test_subset_network.R, test_to_flownetwork.R, test_add_measures.R
• New test file: test_hy_classes.R (374 lines, 25 tests) — constructor validation, auto-classification, producer class stamps, guided error coverage for all dispatch paths, auto-conversion messages, backward compatibility
• Expanded: test_make_index_ids.R (mode/flownetwork/deprecation tests), test_add_levelpaths.R (non-dendritic input), test_add_toids.R (fromnode mutation), test_index.R (ids parameter, 3DHP), test_navigate_hydro_network.R (long-form modes), test_check_hy_graph.R (nested structure)
• Deleted: test_make_fromids.R (folded into test_make_index_ids.R)
• New test data: diversions.csv, diversions.geojson, simple_diversions.geojson, sort_network_dups.rds. Removed: reweight_test.rds