Skip to content

Code Review Guide

Fenris edited this page Jun 6, 2026 · 10 revisions

This guide is for reviewing the PR against the main RavenColonialWeb repo. It is organized as an index so the reviewer can focus on the highest-risk changes first and separate behavior changes from file movement.

1. Start with the shape of the refactor

Recommended files:

  • src/economy/README.md
  • src/economy/index.ts
  • src/economy/system-model2.ts
  • src/economy/economy-model2.ts
  • deleted src/economy-model2.ts
  • moved src/system-model2.ts -> src/economy/system-model2.ts

What to check:

  • Economy and system-model logic now lives under src/economy/.
  • The rest of the app should depend on the economy module, not duplicate economy rules in React views.
  • src/economy/index.ts is the public barrel for economy exports that views need.
  • The move should be mostly organizational, but system-model2.ts also contains behavior changes for link graph handling and economy pre-calculation.

2. Review the system build and link graph first

Recommended file:

  • src/economy/system-model2.ts

Important areas:

  • buildSystemModel2
  • body primary assignment
  • subordinate link assignment
  • calcBodyLinks
  • calcSiteLinks
  • shareColonyLinkPoolFromPrimary
  • calcSiteEconomies
  • ensureSiteEconomiesCalculated

What changed:

  • Body primaries, strong sources, weak sources, same-body weak sources, and subordinate links are now explicitly modeled before economy calculation.
  • Link-source economies are pre-calculated when needed so colony sources expose primaryEconomy.
  • Economy calculation state is tracked as pending, calculating, or done to avoid noisy false warnings during cyclic dependency passes.
  • Economy-capable sites run through a bounded stabilization loop after all source primaries exist, which prevents the returned buildSystemModel2 result from differing from a manual site recalculation.

Review focus:

  • Confirm calcIds is respected for complete-only vs include-incomplete planning mode.
  • Confirm demolish/incomplete sites do not affect completed-only calculations.
  • Confirm link pools are assigned before economies are calculated.
  • Confirm pre-calculation avoids recursion loops and does not change final modeled values unexpectedly.
  • Confirm cross-body weak-link cases are present in the initial rendered model, not only after a manual recalc.

3. Review the per-site economy pipeline

Recommended files:

  • src/economy/economy-model2.ts
  • src/economy/economy-core.ts
  • src/economy/economy-documented.ts

Important areas:

  • calculateColonyEconomies2
  • finishUp
  • adjust
  • applyBodyType
  • applyBuffs
  • applyStrongLinks2
  • applyWeakLinks
  • applyParentHubSubStrongLink

What changed:

  • Economy calculation is now split into smaller rule modules.
  • adjust() remains the central path for mutating economy values and writing audit entries.
  • Strong and weak link application has more explicit handling for colony sources, fixed economies, agriculture sources, and hub/sub-link cases.
  • Neutron-star tourism buff is fixed so it checks neutron-star bodies, not black holes.
  • Agriculture weak-link audit text now says uncapped instead of producing Infinity%.
  • Hightech relay weak links use a stable hightech-anchor pre-scan instead of depending on source name order; medical hightech installations can contribute hightech weak support.

Review focus:

  • Confirm audit output still reflects every applied economy contribution.
  • Confirm body-type intrinsics and buffs still apply in the documented order.
  • Confirm fixed specialized ports only inherit intended economies.
  • Confirm strong-link and weak-link logic does not double-count the same source.
  • Confirm hightech weak-link behavior on mixed relay/medical/source systems, especially star-body outposts vs starports.

4. Review agriculture-specific behavior

Recommended files:

  • src/economy/economy-ag-modifiers.ts
  • src/economy/economy-ag-heuristics.ts
  • src/economy/economy-documented.ts

Important areas:

  • agriculture body modifiers
  • settlement agriculture floor
  • fixed surface/orbital agriculture floors
  • weak-link agriculture budget rules
  • observed preset economy application

What changed:

  • Agriculture rules are separated from generic economy logic.
  • BIO, TIDAL, ICY, ELW/WW, and terraformable agriculture modifiers are handled in dedicated helpers.
  • Heuristic rules are isolated from documented sheet rules so future tuning is easier to review.
  • SystemView has a what-if toggle for terraformable agriculture bonuses. It is off by default, persisted in local storage, and passes enableTerraformableAgricultureBonus into buildSystemModel2.

Review focus:

  • Confirm agriculture-only exceptions are intentionally isolated.
  • Confirm heuristic rules are not leaking into non-agriculture economies.
  • Confirm weak-link caps and floors are visible in audit output.
  • Confirm the terraformable toggle only changes agriculture own-row buffs and strong-link contribution formulas, not unrelated economy rows.

5. Review facility and installation behavior

Recommended files:

  • src/economy/economy-facility-registry.ts
  • src/economy/economy-facilities.ts
  • src/economy/economy-weak-links.ts
  • src/economy/economy-link-sources.ts

What changed:

  • Hubs and installations now use an explicit facility registry.
  • Facilities can be link-only, fixed-intrinsic, or use special Athena hightech behavior.
  • Weak-link eligibility for relay/security/facility cases is handled outside the main calculation pipeline.

Review focus:

  • Confirm every facility build type has the intended registry classification.
  • Confirm link-only facilities do not create market economy rows.
  • Confirm relay/security weak-link filters only apply where intended.
  • Confirm hub child/grandchild strong-link behavior matches the in-game link graph.

6. Review Spansh comparison as UI-only

Recommended files:

  • src/economy/compare/spansh-economy-resolve.ts
  • src/economy/compare/spansh-compare-reliability.ts
  • src/components/SpanshCompareCaveat.tsx
  • src/views/SystemView2/EconomyTable2.tsx
  • src/views/SystemView2/SystemView2.tsx
  • src/api/edsm.ts

What changed:

  • Spansh/EDSM matching helpers were added for comparing modeled RC economies against external market rows.
  • Comparison reliability labels were added so the UI can show when a comparison is meaningful, limited, or excluded.
  • Spansh comparison is display-only and should not feed back into buildSystemModel2 or economy values.
  • Forced Spansh refresh failures now clear loading state.

Review focus:

  • Confirm compare helpers do not mutate modeled economy values.
  • Confirm construction placeholders and excluded rows are handled gracefully.
  • Confirm UI loading state cannot get stuck after a failed forced refresh.
  • Confirm caveat text appears only for comparison reliability, not as model output.

7. Review app import rewiring

Recommended areas:

  • src/views/SystemView2/*
  • src/views/ProjectView/ProjectView.tsx
  • src/components/*
  • src/api/v2-system.ts
  • src/types2.ts

What changed:

  • Many files now import from src/economy, src/economy/system-model2, or src/economy/compare.

Review focus:

  • Confirm imports point to the new module locations.
  • Confirm views are still rendering model output rather than reimplementing rules.
  • Confirm API/type files only reuse shared types and do not create runtime cycles.

8. Review documentation and repo hygiene

Recommended files:

  • docs/economy-model.md
  • docs/economy-model-guide.md
  • src/economy/README.md
  • .gitignore
  • package.json

What changed:

  • New economy documentation explains the production calculation pipeline and plain-language model behavior.
  • Visual Studio workspace files were removed from source control.
  • npm run test:economy targets tracked economy tests under src/economy.

Review focus:

  • Confirm docs describe current behavior rather than aspirational behavior.
  • Confirm local/private fixture references are not required for PR validation.
  • Confirm ignored local tooling is not part of the package scripts.

9. Suggested review order

  1. Skim src/economy/README.md to understand the new module boundaries.
  2. Review system-model2.ts for link graph and calculation ordering.
  3. Review economy-model2.ts, economy-documented.ts, and economy-core.ts for actual percentage changes.
  4. Review agriculture helpers and facility registry for rule-specific behavior.
  5. Review Spansh comparison helpers and SystemView UI changes.
  6. Review import rewiring and docs after the behavior is understood.

10. Suggested test plan for reviewers

These checks only use tracked repo files:

npm test -- --watchAll=false
npm run test:economy -- --runInBand
npm run build
git diff --check

Expected notes:

  • npm test and npm run test:economy should pass the tracked economy regression suite.
  • npm run build should compile successfully.
  • The build may print the existing CRA/Node fs.F_OK deprecation warning from the toolchain.
  • On Windows, git diff --check may print LF-to-CRLF notices for touched files while still exiting successfully.

11. High-value manual smoke checks

If the reviewer wants to run the app locally:

  • Open a system with completed colony sites and verify the economy table still renders percentages and audit rows.
  • Toggle include-incomplete planning mode and confirm planned/building sites affect economy output only in that mode.
  • Toggle Terraformable Agri Bonuses on a terraformable body and confirm agriculture audit rows/formulas change, then toggle it back off to return to conservative live-game estimates.
  • Force a Spansh comparison refresh and confirm loading state clears on success or failure.
  • Inspect Market Links for a body with multiple ports, hubs, or installations and confirm strong/weak sources look plausible.
  • Check a neutron-star system with tourism output and confirm the neutron-star tourism buff appears independently from black-hole logic.

Clone this wiki locally