Change event sorting to event id

[Assem-Uber]

Summary:
Event groups sorting now is based on last event time. This is causing some inconveniance because of the following:

• Hard to traverse the history with that sorting
• Event groups location is changing based on the newly loaded events.
• Pending events sometimes shows after the completion event
• Scrolling to event is programatically can be hard because of change groups changing index.

Changes:

• Extracted firstEventId for groups to be more representable than group id.
• Change sorting logic to depend on numerical value of firstEventId

Screenshots:

[Copilot]

Pull Request Overview

This PR changes the workflow history event sorting mechanism from time-based to event ID-based sorting to improve user experience and fix issues with dynamic group positioning.

• Adds firstEventId field to history group types and implementations
• Replaces time-based sorting with numeric event ID sorting using a new helper function
• Updates all test fixtures and mocks to include the new firstEventId field

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
workflow-history.types.ts	Adds firstEventId field to BaseHistoryGroup type definition
workflow-history.tsx	Updates sorting logic to use getSortableEventId instead of timeMs
get-sortable-event-id.ts	New helper function to convert event IDs to sortable numeric values
get-common-history-group-fields.ts	Extracts and assigns firstEventId from the first event in each group
Various test files	Updates test fixtures and mocks to include the new firstEventId field

[Copilot]

The function treats empty strings as falsy but parseInt(' ', 10) would return NaN, which is handled later. However, parseInt('', 10) also returns NaN. The current logic will return MAX_SAFE_INTEGER for empty strings in the first check, but the test expects it to handle whitespace-only strings. Consider using eventId?.trim() to handle whitespace consistently.

Suggested change

	if (!eventId) return Number.MAX_SAFE_INTEGER;
	const numericEventId = parseInt(eventId, 10);
	const trimmedEventId = eventId?.trim();
	if (!trimmedEventId) return Number.MAX_SAFE_INTEGER;
	const numericEventId = parseInt(trimmedEventId, 10);

[Assem-Uber]

This was done to avoid typescript issue about non string values passed to parseInt

[Copilot]

This test expects whitespace-only strings to return MAX_SAFE_INTEGER, but the current implementation only checks for falsy values. A string with spaces is truthy, so it will proceed to parseInt(' ', 10) which returns NaN, then return MAX_SAFE_INTEGER. While the test passes, the logic path is inconsistent with the other empty value checks.

[Assem-Uber]

The confusion is from the way the typescript issue was handled which made a split in the handling but the results are consistent at the end

[adhityamamallan]

Extracted firstEventId for groups to be more representable than group id.

Will we change the existing usages of group ID to use firstEventId instead?

[adhityamamallan]

How does this work for pending events? Will pending event IDs even be passed to this function?

[Assem-Uber]

Pending events are always the second event so the sorting would happen on the scheduled event

[Assem-Uber]

In case we want to have them sorted we can have use computedEventId for sorting too but also change the id to be scheduledId-Pending instead of Pending-scheduledId so that it will be ordered by id and ignore the string postfix -Pending