-
Notifications
You must be signed in to change notification settings - Fork 0
Code Review Guide
This guide is for reviewing the current RavenColonialWeb economy/SystemView branch against main. It is organized by risk area so reviewers can separate model behavior, UI behavior, and review-only diagnostics.
- Economy code was moved into
src/economy/with clearer module boundaries for system assembly, per-site calculation, agriculture, weak links, facilities, and Spansh compare helpers. - System primary port remains backend-compatible: it is inferred from
system.sites[0]; no persistedprimaryPortIdschema field is introduced. - Order for Calculations is now informational and grouped by body. Drag/drop ordering was removed; changing the system primary port is an explicit picker action that moves the selected site to row 0 on save.
- Tier-point math is decoupled from visual row order by using a canonical tax order.
- Bad imported rows are forced below a
BROKEN BELOWline and excluded from calculations. - Spansh compare remains UI-only, with same-body market inversion hints and an Auto re-order markets action for those hints only.
- System development buff/nerf experimental UI was removed from the main System Stats panel; the commander setting remains as a developer/testing override.
Recommended files:
src/economy/system-model2.tssrc/economy/economy-model2.tssrc/economy/economy-core.tssrc/economy/economy-documented.tssrc/economy/site-calc-exclusions.tssrc/economy/README.mddocs/economy-model.md
Review focus:
-
buildSystemModel2remains the source of truth for calculated system output. - React views render model output and UI state; they should not duplicate economy or tier-point rules.
-
calcIdsis the inclusion boundary for economies, links, tier gives, effects, and unlocks. - No backend schema change is required for primary port, inversion state, or buff/nerf behavior.
Recommended files:
src/economy/system-model2.tssrc/views/SystemView2/BuildOrder.tsxsrc/views/SystemView2/build-order-sort.tssrc/views/SystemView2/SystemView2.tsxsrc/views/SystemView2/SystemCard.tsx
Important behavior:
- System primary port is inferred from
system.sites[0]. -
buildSystemModel2copies that inferred id into runtime-onlysysMap.primaryPortId. - If
sites[0]is invalid, the runtime model falls back to the first valid complete starport/outpost, then tosites[0]. - Saving a changed primary port moves the selected eligible port to
system.sites[0]. -
SystemCardstrips runtime-onlyprimaryPortIdbefore save. - Body market-link primaries are computed from eligible calculated sites on each body, not from a new persisted field.
Review focus:
- Confirm no
primaryPortIdis sent to the backend. - Confirm saved primary port survives reload because it occupies
system.sites[0]. - Confirm Order panel preview math uses the current displayed row order after a primary-port change.
- Confirm body primary icons and market-link icons track model-derived body primaries.
Recommended files:
src/economy/site-calc-exclusions.tssrc/economy/system-model2.tssrc/views/SystemView2/build-order-sort.tssrc/views/SystemView2/BuildOrder.tsx
Important behavior:
- Completed-only mode includes complete sites only.
- Use all Sites includes non-demolished sites up to
idxCalcLimit. - Unknown body, unknown/null build type, and positive single-digit
marketIdvalues (1through9) are excluded regardless of status. - Excluded rows are displayed below
BROKEN BELOWso users can identify bad import data.
Review focus:
- Confirm excluded rows do not enter
calcIds. - Confirm excluded rows remain below the cut line on initial page load, not only after toggling modes.
- Confirm excluded complete sites do not affect tier points, economies, links, system effects, or unlocks.
Recommended files:
src/views/SystemView2/BuildOrder.tsxsrc/views/SystemView2/build-order-sort.tssrc/views/SystemView2/SystemView2.tsx
Important behavior:
- The panel groups rows by body for readability.
- Rows are not draggable.
-
Completed sites onlyandUse all Sitesmirror the main SystemView toggle. - Use all Sites places non-planned sites first, then planned sites, while preserving body grouping inside each section.
- Group by body preserves the active cut line.
- A primary-port picker appears over the panel and is populated by eligible orbital ports with landing pads, including orbital outposts.
Review focus:
- Confirm initial panel state matches the main page toggle.
- Confirm clicking mode buttons in either place updates the same
useIncompletestate. - Confirm
Okayrebuilds using the panel-selected mode and order. - Confirm the primary-port picker does not close the parent panel when interacted with.
- Confirm the table has no unnecessary horizontal scrollbar.
Recommended file:
src/economy/system-model2.ts
Important behavior:
- Tier-point totals use canonical tax order, not visual order.
- The system primary port is skipped for tier needs.
- Taxed starports sort by site tier descending, body order, orbital before surface,
marketId, then name/id. - The tax amount still uses
site.type.tier, matching deployed behavior. - Planned-site insufficient-point warnings are UI warnings only.
Review focus:
- Confirm visual grouping does not change
tierPoints. - Confirm
calcNeedsis cleared and rebuilt every pass. - Confirm completed-only vs Use all Sites respects
calcIdsand status. - Confirm red insufficient-point warnings appear only for planned sites, not complete/building sites.
- Confirm HIP 52675 completed-only reference remains
Tier 2 = 0,Tier 3 = +12.
Recommended files:
src/economy/system-model2.tssrc/views/SystemView2/SystemStats.tsxsrc/components/ModalCommander.tsxsrc/views/ProjectView/ProjectView.tsx
Important behavior:
- The public System Stats experimental checkbox was removed.
- The commander settings checkbox remains as a developer/testing override.
- The UI label is
Apply system development buff/nerf model. - Primary-port buffing follows
sysMap.primaryPortId, derived fromsystem.sites[0].
Review focus:
- Confirm ProjectView and SystemView interpret
noBuffNerfconsistently. - Confirm disabling the override is only an old-model comparison path.
- Confirm no backend schema change was added.
Recommended files:
src/economy/system-model2.tssrc/economy/economy-model2.tssrc/economy/economy-documented.tssrc/economy/economy-link-sources.tssrc/economy/economy-weak-links.ts
Important behavior:
- Body primaries and subordinate links are assigned before link pools are built.
- Economy-capable sources are calculated on demand so
primaryEconomyis available. - Economy calculation state guards against cyclic pre-calculation.
- Stabilization reruns economy/link summaries until output stops changing or the pass limit is reached.
- Parent-hub sub-strong extra contribution is intentionally disabled for current observed-data parity.
Review focus:
- Confirm
calcIdsis respected during link graph construction and calculation. - Confirm flattened hub grandchildren do not double-count.
- Confirm gas-giant cluster farms strong-link only the body primary.
- Confirm relay/security/medical/farm weak-link rules remain isolated in economy weak-link modules.
Recommended files:
src/economy/economy-ag-modifiers.tssrc/economy/economy-ag-heuristics.tssrc/economy/economy-facility-registry.tssrc/economy/economy-facilities.ts
Important behavior:
- Terraformable agriculture is controlled by
enableTerraformableAgricultureBonus. - Historical agriculture weak-link budget rule matching is disabled for live calculations.
- Only the tidal orbital cluster agriculture weak-link cap remains active.
- Settlement fixed economy is currently
1.0. -
athenacan be 100% or 140% High Tech depending on qualifying comms. - Facility High Tech rows skip BIO/GEO own-row High Tech buffs.
Review focus:
- Confirm old fitted AG budget constants are not accidentally wired back into live calculations.
- Confirm
explainAgricultureWeakLinkBudgetremains diagnostic-only. - Confirm facility registry entries match intended link-only, fixed, or
athenaCommsbehavior. - Confirm link-only facilities do not create market economies.
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 is UI-only and must not feed
buildSystemModel2. - Compare resolution tries RC/journal
marketIdfirst, then EDSM name matching. - No-pad facilities are excluded from hard compare and treated as limited reliability.
- Same-body inversion hints are derived state and are not persisted.
- Auto re-order markets applies current inversion swaps only; it does not run a general regroup.
Review focus:
- Confirm hints disappear when current model/data no longer indicates inversion.
- Confirm hover text names the swap partner and confidence.
- Confirm excluded or informational Spansh rows do not create hard warnings.
- Confirm feedback/report links still open the feedback modal over audit panels.
Recommended files:
src/views/SystemView2/BuildOrder.tsxsrc/views/SystemView2/ViewSite.tsxsrc/components/ShareFeedback.tsxsrc/views/SystemView2/EconomyTable2.tsx
Important behavior:
- Primary port, primary market-link, planned/building/demolished, and inversion icons appear inline after site text.
- Site details panels scroll internally below the top detail section and should not run behind the bottom site banner.
- Feedback supports pasted/dropped/selected image attachments as base64 data URLs.
- The Audit button near economy ratios is styled as a small clickable button.
Review focus:
- Confirm icons fit row height and do not overlap text.
- Confirm scrollbars appear only where useful, with no horizontal scrollbar in normal panel layouts.
- Confirm feedback modal interaction does not freeze the page or close parent panels unexpectedly.
Recommended files:
.gitignorepackage.jsonyarn.lockdocs/economy-model.mddocs/economy-model-guide.mdsrc/economy/README.md
Important behavior:
- The repo remains Yarn-only;
package-lock.jsonshould not be tracked. -
.vscode/should not be tracked. - Generated docs artifacts (
.pdf,.print.html) belong outside tracked live folders. - Runtime docs in
docs/should describe current code, not old hypotheses. - Local-only tests and diagnostics belong under
local/and are not part of deployment.
Review focus:
- Confirm
package.jsonandyarn.lockmatch intentional dependency changes only. - Confirm no local-only files or generated artifacts are staged in tracked folders.
- Confirm docs do not claim disabled AG budget rules, draggable ordering, or
npm run test:economyas live behavior.
- Review
system-model2.tsprimary-port, forced-exclusion, tier-point, system-effect, and link-graph behavior. - Review
BuildOrder.tsxandbuild-order-sort.tsgrouping, cut lines, mode mirroring, icons, and primary-port picker. - Review Spansh compare/inversion code as UI-only derived state.
- Review SystemView and commander setting changes for buff/nerf behavior.
- Review agriculture and facility modules after the model invariants are clear.
- Review docs and repo hygiene last.
Tracked repo checks:
npm run build
git diff --checkExpected 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. - Local-only regression/fixture tests live under
local/economy/tests/and are not part of the tracked package scripts.
- Open
HIP 52675and confirm completed-only tier points match the live reference:Tier 2 = 0,Tier 3 = +12. - Open Order for Calculations in completed-only mode and confirm only completed valid sites are above the cut line.
- Switch to Use all Sites and confirm building sites appear before planned sites, with planned sites grouped last.
- Confirm invalid imported rows appear below
BROKEN BELOWand do not affect totals. - Change the primary port through the picker, save, reload, and confirm the selected site remains
system.sites[0]. - Run whole-system Spansh compare and confirm possible inversion arrows show only as derived hints.
- Press Auto re-order markets and confirm it applies only the hinted inversion swaps.
- 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.
- Open a site detail panel with many economy rows and confirm the body of the panel scrolls without overlapping the bottom banner.