Feat: Trace/chart view, resizable logger columns, trend arrows, stopId display, and OpenTransit green theme#32
Merged
Conversation
…topId display, and green theme
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 41 minutes and 16 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, 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 have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR introduces numeric tolerance for JSON comparison, watched key tracking for log monitoring, stop ID search filtering for GTFS entities, and trace charting with column resizing in the KeyLogViewer. All UI components are rebranded from indigo to green Tailwind theme. ChangesFeature Implementation: Tolerance, Watch Keys, Stop Indexing, and Trace Charting
UI Theme: Indigo to Green Color Scheme
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/lib/components/ProtobufViewer.svelte (1)
817-837:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
trip.stopIdfallback for vehicle stop badge.Line 820 reads only top-level
stopId. For feeds where stop ID is present only attrip.stopId, the new stop badge is missing.Proposed fix
{`@const` vehicle = item as Record<string, unknown>} {`@const` vehicleInfo = vehicle.vehicle as Record<string, string> | undefined} {`@const` trip = vehicle.trip as Record<string, string> | undefined} - {`@const` vehicleStopId = (vehicle as Record<string, string>).stopId} + {`@const` rawVehicle = vehicle as Record<string, unknown>} + {`@const` vehicleStopId = + (typeof rawVehicle.stopId === 'string' ? rawVehicle.stopId : undefined) ?? + trip?.stopId}🤖 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 `@src/lib/components/ProtobufViewer.svelte` around lines 817 - 837, The vehicle stop badge only reads the top-level stopId (variable vehicleStopId) so feeds that put the stop on the trip object (trip.stopId) are missed; update the computation of vehicleStopId (used in ProtobufViewer.svelte around the block where item/vehicle, vehicleInfo, and trip are defined) to fallback to trip?.stopId when (vehicle as Record).stopId is falsy so the badge shows either vehicle.stopId or trip.stopId.src/lib/utils/jsonCompare.ts (2)
196-199:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't reuse the equality cache across tolerance values.
deepEqualIgnoreOrdernow depends onnumericTolerancePercent, butequalityCacheis still keyed only by object identity. A result cached at0tolerance will be reused for5, and vice versa, so the same pair can report stale equality after the user changes the tolerance.Suggested fix
- if (typeof a === 'object' && typeof b === 'object' && ignoredKeys.length === 0) { + if ( + typeof a === 'object' && + typeof b === 'object' && + ignoredKeys.length === 0 && + numericTolerancePercent === 0 + ) { const aCache = equalityCache.get(a as object); if (aCache?.has(b as object)) { return aCache.get(b as object)!; } } @@ - if (typeof a === 'object' && typeof b === 'object' && ignoredKeys.length === 0) { + if ( + typeof a === 'object' && + typeof b === 'object' && + ignoredKeys.length === 0 && + numericTolerancePercent === 0 + ) { let aCache = equalityCache.get(a as object); if (!aCache) { aCache = new WeakMap(); equalityCache.set(a as object, aCache); }Also applies to: 243-250
🤖 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 `@src/lib/utils/jsonCompare.ts` around lines 196 - 199, The equality cache (equalityCache) is being reused across different numericTolerancePercent values causing stale results in deepEqualIgnoreOrder; update the caching strategy so cache entries are partitioned by the tolerance (e.g., include numericTolerancePercent in the cache key or maintain a top-level map from tolerance to equalityCache) and apply the same change in both places noted (around the blocks referencing equalityCache at the shown lines and at 243-250). Ensure the lookup and set operations on equalityCache incorporate the tolerance value (and still use object identity for the inner keys) so cached comparisons are only reused when the tolerance matches.
273-349:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
countDifferencesstill ignores tolerance for normal-sized arrays.Once the code leaves the large-array sampling path, it falls back to
stableStringify+ string comparison. That means a tolerated change like[100]vs[104]at5%still increments the diff count, so collapsed badges can disagree withgetDiffStatus/deepEqualIgnoreOrderfor the same data.🤖 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 `@src/lib/utils/jsonCompare.ts` around lines 273 - 349, countDifferences currently compares non-large arrays by stableStringify then simple string equality, which ignores numericTolerancePercent; change the array comparison so mismatched sorted entries (aSorted/bSorted) are further compared using deepEqualIgnoreOrder on the original items with numericTolerancePercent (use the original arrays a and b mapped to the same sort order or keep parallel arrays of {key:string,item} so you can call deepEqualIgnoreOrder(itemA,itemB,ignoredKeys,numericTolerancePercent)); if deepEqualIgnoreOrder returns true treat them as equal (advance both pointers) otherwise count a difference and advance the lesser entry as before. Ensure you reference the existing symbols aSorted, bSorted, stableStringify, sortById, deepEqualIgnoreOrder and the parameters numericTolerancePercent when implementing the change.
🤖 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 `@src/lib/components/JsonTree.svelte`:
- Around line 151-160: Normalize array-index segments to wildcard form before
storing and checking watched keys: when building watchedKeySet from
comparatorState.watchedKeysInput, map each entry to a normalized form by
replacing any bracketed numeric indices (e.g., [0], [12]) with .*, trimming
redundant dots/leading dots so keys like "foo[3].bar" become "foo.*.bar";
likewise compute isWatched against a normalized currentPath (replace numeric [n]
segments with .* before calling watchedKeySet.has). Apply the same normalization
logic to the parallel code block referenced around lines 310-319 so both storage
and lookup use the same wildcard-normalized paths.
In `@src/lib/components/KeyLogViewer.svelte`:
- Around line 104-110: The current construction of allRawVals (from
entries.flatMap and the server1_value/server2_value arrays) uses Number(...)
which coerces null, empty strings, booleans and numeric strings into values and
produces false numeric points; instead, only include genuine numeric values:
when flattening server1_value and server2_value into a1/a2, filter each
candidate value by checking typeof value === 'number' and Number.isFinite(value)
(or parse the value and only accept if it parses to a finite number and original
was numeric string if you want to accept strings), then map to Number(value)
only for those accepted; apply the same numeric-only guard to the other similar
blocks that build numeric arrays (the analogous flatMap/map sections referenced
at the later blocks using server1_value/server2_value).
- Around line 113-116: The lint failure is from unused declarations: remove the
unused colors parameter from the buildSeries function signature and body (and
update any callers of buildSeries to stop passing a colors argument), and delete
the unused COLUMN_KEYS constant (and any related dead lines referenced around
165-166) so there are no leftover unused symbols; if you prefer wiring them
instead, use COLUMN_KEYS to drive the series labels inside buildSeries and
consume the colors array to attach a color property to each series, but keep
changes consistent by updating callers and tests accordingly.
- Around line 724-731: The trace selector is currently built from filteredLogs
which hides valid key paths; change it to derive unique key paths from the full
log source (e.g., logs or allLogs) instead of filteredLogs so the picker is
independent of table row limits and filters; update the select's each block to
use [...new Set(<fullLogArray>.map(l => l.key_path))] and keep
lastPathSegment(kp) for labels, ensuring traceKeyPath binding and any null/empty
checks still work.
- Around line 395-416: The fetchChartLogs routine can assign stale results after
the user changes loggerState.selectedEndpoint or traceKeyPath; update
fetchChartLogs and the $effect to prevent this by capturing the current
selection before the async fetch (e.g., save const currentEndpoint =
loggerState.selectedEndpoint and const currentKey = traceKeyPath or use an
AbortController/requestId) and, after awaiting the response, verify the saved
selection still matches loggerState.selectedEndpoint and traceKeyPath before
setting chartLogs; if they differ, discard the response (or abort the request)
so chartLogs and chartData are never overwritten by stale responses.
In `@src/lib/components/ProtobufPanel.svelte`:
- Line 415: The button's dark-mode hover class is left as blue; update the class
string in ProtobufPanel.svelte (the element with class="text-sm font-medium
text-green-600 transition-colors hover:text-green-700 dark:text-green-400
dark:hover:text-blue-300") by replacing dark:hover:text-blue-300 with
dark:hover:text-green-300 so the dark-mode hover matches the green accent
palette used by dark:text-green-400 and hover:text-green-700.
In `@src/lib/components/ProtobufViewer.svelte`:
- Around line 561-565: The raw-text tab still uses the blue hover utility; in
ProtobufViewer.svelte locate the tab element whose class string contains
"activeRawTextTab === tab.id" and replace the "hover:bg-blue-50" token with the
green variant (e.g., "hover:bg-green-50") so the hover color matches the
existing green theme classes (bg-green-500, text-green-700, etc.).
- Around line 773-777: The badge currently reads stopTimeUpdates?.[0]?.stopId
which only uses the first array element; change tripStopId so it finds the first
stopTimeUpdate entry with a defined stopId (e.g., use Array.prototype.find on
stopTimeUpdates to locate the first item where stopId is truthy) and extract
that stopId instead of always using index 0; update the const tripStopId
declaration (related to stopTimeUpdates and tripUpdate) to reflect this change
so badges show when a later entry has a stopId.
In `@src/lib/components/SplitPane.svelte`:
- Line 62: The class directive uses the old v3 prefix form; update the dragging
class in SplitPane.svelte to use Tailwind v4’s trailing important suffix and a
valid Svelte class key, e.g. replace class:!bg-green-500={isDragging} with a
suffix form such as class:{"bg-green-500!"}={isDragging} (or
class:bg-green-500!={isDragging} if your linter/parser accepts it) so the
utility uses the v4 "!" important modifier while still referencing the
isDragging boolean.
In `@src/lib/utils/search.ts`:
- Around line 127-135: For type 'vehiclePositions' in the search indexing block,
also index the nested trip stop id so vehicle records with stop id under
vehicle.trip.stopId are discoverable: call addToIndex on index.stopIds with
vehicle.trip?.stopId (in addition to the existing (e as
Record<string,string>).stopId) inside the same branch that handles
'vehiclePositions' (refer to addToIndex, index.stopIds, vehicle.trip?.stopId,
vehicle.stopId, and the 'vehiclePositions' branch).
---
Outside diff comments:
In `@src/lib/components/ProtobufViewer.svelte`:
- Around line 817-837: The vehicle stop badge only reads the top-level stopId
(variable vehicleStopId) so feeds that put the stop on the trip object
(trip.stopId) are missed; update the computation of vehicleStopId (used in
ProtobufViewer.svelte around the block where item/vehicle, vehicleInfo, and trip
are defined) to fallback to trip?.stopId when (vehicle as Record).stopId is
falsy so the badge shows either vehicle.stopId or trip.stopId.
In `@src/lib/utils/jsonCompare.ts`:
- Around line 196-199: The equality cache (equalityCache) is being reused across
different numericTolerancePercent values causing stale results in
deepEqualIgnoreOrder; update the caching strategy so cache entries are
partitioned by the tolerance (e.g., include numericTolerancePercent in the cache
key or maintain a top-level map from tolerance to equalityCache) and apply the
same change in both places noted (around the blocks referencing equalityCache at
the shown lines and at 243-250). Ensure the lookup and set operations on
equalityCache incorporate the tolerance value (and still use object identity for
the inner keys) so cached comparisons are only reused when the tolerance
matches.
- Around line 273-349: countDifferences currently compares non-large arrays by
stableStringify then simple string equality, which ignores
numericTolerancePercent; change the array comparison so mismatched sorted
entries (aSorted/bSorted) are further compared using deepEqualIgnoreOrder on the
original items with numericTolerancePercent (use the original arrays a and b
mapped to the same sort order or keep parallel arrays of {key:string,item} so
you can call
deepEqualIgnoreOrder(itemA,itemB,ignoredKeys,numericTolerancePercent)); if
deepEqualIgnoreOrder returns true treat them as equal (advance both pointers)
otherwise count a difference and advance the lesser entry as before. Ensure you
reference the existing symbols aSorted, bSorted, stableStringify, sortById,
deepEqualIgnoreOrder and the parameters numericTolerancePercent when
implementing the change.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fa3a8a1a-5400-48ef-82c6-11fb9ad2410f
📒 Files selected for processing (17)
src/app.csssrc/lib/components/ComparatorPanel.sveltesrc/lib/components/DiffViewer.sveltesrc/lib/components/GtfsRtLogViewer.sveltesrc/lib/components/JsonDiff.sveltesrc/lib/components/JsonTree.sveltesrc/lib/components/JsonViewer.sveltesrc/lib/components/KeyLogViewer.sveltesrc/lib/components/ProtobufPanel.sveltesrc/lib/components/ProtobufViewer.sveltesrc/lib/components/SimpleJsonTree.sveltesrc/lib/components/SplitPane.sveltesrc/lib/components/ToolsPanel.sveltesrc/lib/panelState.svelte.tssrc/lib/utils/jsonCompare.tssrc/lib/utils/search.tssrc/routes/+page.svelte
…ality cache in jsonCompare, and add stopId to GTFSVehiclePosition
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Major enhancement to the maglev-validator with a trace/chart view for watched keys, resizable logger table columns, trend arrows, stopId display in GTFS-RT viewer, and OpenTransit green theme replacement.
Changes
deepEqualIgnoreOrdersorts objects by ID before comparison. Logger array detail modal usessortById.#78aa36added as custom color.max-h-[65vh]with internal scroll.Summary by CodeRabbit
Release Notes
New Features
Style Updates