feat: add schematic map reference inventory#257
Conversation
|
Warning Review limit reached
More reviews will be available in 33 minutes and 49 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis pull request adds Phase 1 schematic map reference infrastructure: a documentation page, an inventory generation CLI script that scans and analyzes map components to produce structured snapshots with element counts and layer ordering, and a topology derivation CLI script that computes service coverage at a specific timestamp and optionally validates against the inventory. ChangesSchematic Map Reference Inventory and Topology Tools
Sequence Diagram(s)sequenceDiagram
participant Inventory as Inventory<br/>Generator
participant MapDir as Map Component<br/>Directory
participant Topology as Topology<br/>Derivation
participant DataDir as Service Data<br/>Directory
participant Comparison as Inventory<br/>Comparison
rect rgba(100, 150, 200, 0.5)
note over Inventory,MapDir: Snapshot Generation
Inventory->>MapDir: scan Map*.tsx files
MapDir-->>Inventory: filenames + content
Inventory->>Inventory: extract date, parse SVG
Inventory->>Inventory: count elements, categorize ids
Inventory-->>Inventory: produce inventory JSON
end
rect rgba(150, 200, 100, 0.5)
note over Topology,DataDir: Topology Derivation
Topology->>DataDir: load line, station, service
DataDir-->>Topology: JSON entities
Topology->>Topology: compute active at --at time
Topology->>Topology: aggregate stations & segments
Topology-->>Topology: produce topology JSON
end
rect rgba(200, 150, 100, 0.5)
note over Comparison: Optional Validation
Comparison->>Comparison: compare derived vs inventory
Comparison-->>Comparison: report mismatches
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
3383693 to
0dd85bf
Compare
0dd85bf to
45d4e36
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
scripts/schematic-map-inventory.mjs (1)
259-259: 💤 Low valueConsider clarifying the deterministic timestamp.
Using
new Date(0)produces a 1970 epoch date, which is intentional for deterministic output. However,generatedAtsuggests the actual generation time. Consider adding a brief comment or renaming tobaselineTimestampto clarify the intent.- generatedAt: new Date(0).toISOString(), + generatedAt: new Date(0).toISOString(), // fixed for deterministic output🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/schematic-map-inventory.mjs` at line 259, The property generatedAt currently set to new Date(0).toISOString() is misleading because it uses the epoch as a deterministic baseline; update the code to either rename the property (e.g., baselineTimestamp or deterministicTimestamp) or add an inline comment next to generatedAt explaining that new Date(0) is intentional to produce a deterministic epoch timestamp for reproducible output (reference the generatedAt property and the new Date(0).toISOString() call so you change the correct spot).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/schematic-map-reference-inventory.md`:
- Around line 64-65: The doc has a mismatch for the 2025-04 map: Line 31 states
214 line segments but the section text states 213 station-to-station ids; update
the 2025-04 segment-count so both places match or add a clear sentence
explaining why one segment is excluded from the station-to-station id count
(e.g., a non-station connector, loop, or metadata-only segment). Locate
occurrences of "2025-04", the "214" count on line 31, and the phrase "213
station-to-station ids" and either change the numeric value to be consistent or
append the explanatory note referencing "station-to-station ids" so the
discrepancy is documented.
In `@scripts/schematic-map-topology.mjs`:
- Around line 60-64: Add a fail-fast validation that enforces --inventory and
--effective-date are provided together: if one of options.inventory or
options.effectiveDate is set but the other is missing, throw a descriptive Error
(e.g. "'--inventory' and '--effective-date' must be provided together") so
comparison cannot be silently disabled; apply the same check in both places
referenced around the current options.at validation and the duplicate block at
the later location (lines ~293-300).
- Around line 83-89: The timestamp() function currently normalizes some date
forms and then calls Date.parse(normalized), which will treat ISO-like strings
without a timezone (e.g. "2025-04-01T00:00:00") as local time and produce
non-deterministic results; update timestamp() to detect ISO datetime strings
that include time but lack a timezone designator (match patterns like
/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[+\-]\d{2}:\d{2})?$/ and
reject when the timezone part is missing) and throw/return an error indicating a
missing timezone instead of calling Date.parse, then only call
Date.parse(normalized) for inputs that either were normalized with an explicit
timezone or already include a timezone designator; ensure the check is applied
inside the timestamp function before parsing.
---
Nitpick comments:
In `@scripts/schematic-map-inventory.mjs`:
- Line 259: The property generatedAt currently set to new Date(0).toISOString()
is misleading because it uses the epoch as a deterministic baseline; update the
code to either rename the property (e.g., baselineTimestamp or
deterministicTimestamp) or add an inline comment next to generatedAt explaining
that new Date(0) is intentional to produce a deterministic epoch timestamp for
reproducible output (reference the generatedAt property and the new
Date(0).toISOString() call so you change the correct spot).
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: db59d076-dc00-43cc-818b-20b293e9edd5
📒 Files selected for processing (4)
docs/schematic-map-reference-inventory.jsondocs/schematic-map-reference-inventory.mdscripts/schematic-map-inventory.mjsscripts/schematic-map-topology.mjs
Summary
mrtdown-siteMap*.tsxinventory extractor for schematic map Phase 1 planning2025-04baseline comparison and the distinction between operational graph coverage and displayed future or under-construction schematic coverageValidation
npm run checkSummary by CodeRabbit
Release Notes
Documentation
Chores