forked from njthomson/RavenColonialWeb
-
Notifications
You must be signed in to change notification settings - Fork 0
Code Review Guide
Fenris159 edited this page Jun 13, 2026
·
10 revisions
This guide is for reviewing the current RavenColonialWeb economy/SystemView changes. It is organized by risk area so reviewers can separate model behavior from UI-only diagnostics and local-only test tooling.
Recommended files:
src/economy/system-model2.tssrc/economy/economy-model2.tssrc/economy/economy-core.tssrc/economy/economy-documented.tssrc/economy/README.md
Review focus:
-
buildSystemModel2remains the source of truth for calculated system output. - React views should render model output, not duplicate economy or tier-point rules.
-
system.sites[0]is still the only persisted system primary-port source. No new backend field should be required. - Local test files under
local/are developer-only and are not deployed.
Recommended files:
src/economy/system-model2.tssrc/views/SystemView2/BuildOrder.tsxsrc/views/SystemView2/SystemView2.tsx
Important behavior:
- System primary port is inferred from
system.sites[0]. -
buildSystemModel2copies that intosysMap.primaryPortIdfor runtime use. - Saving the Order panel persists the reordered IDs back to
system.sites. - Moving a different site into row 1 and saving should make that site the system primary after reload.
- Body market-link primaries are inferred by the model from body site order, eligible type, tier, and calculation inclusion.
Review focus:
- Confirm no new
primaryPortIdschema value is saved to the backend. - Confirm
sites[0]survives load, reorder, save, and reload as the primary-port source. - Confirm same-body custom ordering is preserved within computed primary-rank groups.
- Confirm market ID is not used as the within-body ordering fallback in the Order panel.
Recommended files:
src/views/SystemView2/BuildOrder.tsxsrc/views/SystemView2/SystemView2.tsxsrc/economy/system-model2.ts
Important behavior:
- The Order panel groups rows by body for accessibility.
- The system primary port remains first when it is present in the relevant section.
- Sites below the cut line remain below the cut line.
-
Completed sites onlyputs only completed sites above the cut line. -
Use all Sitesputs non-planned sites first, then planned sites, while keeping grouping inside each section. - Build/in-progress sites do not appear above the cut line in completed-only mode, but they appear before planned sites in the table.
Review focus:
- Confirm initial panel load mirrors the current main SystemView completed/all-sites toggle.
- Confirm clicking the panel
Completed sites onlyandUse all Sitesbuttons updates the sameuseIncompletestate as the main toolbar toggle. - Confirm pressing
Okayrebuilds using the mode selected inside the panel. - Confirm planned sites are not interleaved above completed/building sites when all sites are included.
Recommended file:
src/economy/system-model2.ts
Important behavior:
- Tier-point totals now use a canonical tax order so UI row grouping does not change math.
- Taxed starports are ordered by site tier descending, then body order, orbital before surface, market ID, then name/id fallback.
- The actual tax percentage still uses
site.type.tier, matching the deployed live behavior. - HIP 52675 should match the live deployed reference of
Tier 2 = 0,Tier 3 = +12.
Review focus:
- Confirm reordering rows for display does not change
tierPoints. - Confirm
calcNeedsis cleared and rebuilt every calculation pass. - Confirm completed-only vs all-sites still respects
calcIdsand build/plan status. - Confirm red insufficient-points warnings in the Order panel appear only for planned sites, not completed/building sites.
Recommended files:
src/economy/system-model2.tssrc/views/SystemView2/SystemStats.tsxsrc/components/ModalCommander.tsxsrc/views/ProjectView/ProjectView.tsx
Important behavior:
- The public System Stats checkbox and experimental wording were removed.
- Buff/nerf is treated as normal system development model behavior.
- The commander settings checkbox remains as a developer/testing override.
- The UI label is
Apply system development buff/nerf model. - The testing override should normally remain enabled.
- The initial buff is tied to
sysMap.primaryPortId, which is derived fromsystem.sites[0], not the first visible/included row.
Review focus:
- Confirm disabling the commander testing override is only useful for comparing the old pre-balance model.
- Confirm
ProjectViewinterpretsnoBuffNerfconsistently. - Confirm the initial starport buff follows a saved primary-port reorder after reload.
- Confirm no backend schema change was added for this behavior.
Recommended files:
src/economy/system-model2.tssrc/economy/economy-model2.tssrc/economy/economy-core.tssrc/economy/economy-documented.tssrc/economy/economy-link-sources.tssrc/economy/economy-weak-links.ts
Important behavior:
- Body primaries, subordinate links, strong sources, weak sources, and same-body weak sources are modeled before economy calculation.
- Economy-source sites are pre-calculated when needed so
primaryEconomyis available. - Economy calculation state prevents cyclic pre-calculation noise.
- The stabilization pass keeps initial render output aligned with manual recalculation.
Review focus:
- Confirm
calcIdsis respected for complete-only vs all-sites mode. - Confirm link pools are assigned before dependent economies are calculated.
- Confirm strong/weak links do not double-count sources.
- Confirm body primary changes caused by custom same-body order produce expected market-link primaries.
Recommended files:
src/economy/economy-ag-modifiers.tssrc/economy/economy-ag-heuristics.tssrc/economy/economy-facility-registry.tssrc/economy/economy-facilities.ts
Review focus:
- Confirm agriculture-specific exceptions remain isolated.
- Confirm terraformable agriculture is controlled by
enableTerraformableAgricultureBonus. - Confirm hub/installation registry classifications match intended link-only, fixed-intrinsic, and special-case behavior.
- Confirm facility rows do not accidentally create market economies when they should only act as link sources.
Recommended files:
src/economy/compare/spansh-economy-resolve.tssrc/economy/compare/spansh-compare-reliability.tssrc/economy/compare/spansh-inversion-detect.tssrc/views/SystemView2/AuditTestWholeSystem.tsxsrc/views/SystemView2/BuildOrder.tsxsrc/views/SystemView2/EconomyTable2.tsx
Important behavior:
- Spansh compare remains UI-only and must not feed back into
buildSystemModel2. - Same-body inversion hints are derived from current audit data and are not persisted.
- Exact swapped economy matches get highest confidence.
- Strong stale-data hints require a large swapped-match improvement and can use market-link-primary economy-count evidence.
- The Order panel shows up/down hint icons after existing row icons, with hover reasons.
Review focus:
- Confirm inversion hints disappear after the current model/data no longer indicates inversion.
- Confirm excluded or informational Spansh rows do not create hard inversion warnings.
- Confirm hover text names the swap partner and explains exact or strong confidence.
- Confirm no backend field stores inversion state.
Recommended file:
src/views/SystemView2/BuildOrder.tsx
Important behavior:
- Primary port icon still appears on row 1.
- Primary market-link icon appears for body orbital/surface primary sites.
- Planned/building/demolished icons still appear after the site description.
- Spansh inversion arrows appear after existing row icons.
Review focus:
- Confirm icons fit the row height and do not overlap row text.
- Confirm hover/title text is useful and not misleading.
- Confirm cutoff rows and greyed-out rows still render legibly.
Recommended locations:
src/economy/*.test.tslocal/economy/testslocal/economy/fixtures
Important behavior:
- Tracked tests under
src/economyare normal repo tests. - Tests under
local/economy/testsare developer-only and ignored by git/deployment. - Local HIP 52675 tests compare grouping stability, tier-point parity with the deployed formula, and primary-port buff behavior.
- Local Spansh inversion tests cover exact swaps, strong stale-data swaps, and non-inversion stale mismatches.
Review focus:
- Confirm production code needed by the live app is under
src, notlocal. - Confirm local-only fixtures are not required for deployment.
- Confirm new model changes have either tracked tests or local regression coverage appropriate to the risk.
- Review
system-model2.tsprimary-port, tier-point, system-effect, and link-graph changes. - Review
BuildOrder.tsxgrouping, mode mirroring, icon rendering, and saved-order preservation. - Review Spansh inversion detection as UI-only derived state.
- Review SystemView and commander setting changes for buff/nerf behavior.
- Review economy rule modules only after the ordering/model invariants are clear.
- Review tests and docs last.
Tracked repo checks:
npm run test:economy
npm run build
git diff --checkLocal developer checks:
npx jest --config local/jest.config.js --testPathPattern="hip52675-(tierpoints-live-compare|grouping-experiment)|primary-port-buff-nerf" --runInBand
npx jest --config local/jest.config.js --testPathPattern=spansh-inversion-detect --runInBandExpected notes:
-
npm run buildmay print the existing CRA/Nodefs.F_OKdeprecation warning. - On Windows,
git diff --checkmay print LF-to-CRLF notices for touched files while still exiting successfully. - Full local Jest may include older local-only experiments; prefer focused local patterns unless intentionally auditing all local tests.
- Open
HIP 52675and confirm tier points match the live reference:Tier 2 = 0,Tier 3 = +12. - Open Order for Calculations in completed-only mode and confirm only completed sites are above the cut line.
- Switch to Use all Sites and confirm building sites appear before planned sites, with planned sites grouped last.
- Reorder same-body market-link candidates, save, reload, and confirm the inferred primary market link follows the saved order.
- Run whole-system Spansh compare and confirm possible inversion arrows show only as derived hints.
- Hover an inversion arrow and confirm the reason names the partner and confidence.
- Confirm System Stats no longer shows the old experimental buff/nerf checkbox.
- In commander settings, confirm the buff/nerf checkbox reads as a developer/testing override.