-
Notifications
You must be signed in to change notification settings - Fork 0
Release/d #26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Release/d #26
Changes from 24 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
aadb7da
chore: prepare Release D (v2.9.0)
magmacomputing 79d3cac
fix doco
magmacomputing e462af7
1st draft PR
magmacomputing 2af7698
2nd draft PR review
magmacomputing 945d845
3rd draft PR review
magmacomputing a22e7c7
hasOwn
magmacomputing fed3f71
all hasOwn
magmacomputing ef10d08
ci.yml
magmacomputing 0e6a546
aliasEngine heirarchy
magmacomputing 1d3e29e
Alias warning via Logify
magmacomputing 1e60a99
extending AliasEngine
magmacomputing e83c8d2
monday 6:22
magmacomputing 74ae34b
my AliasEngine
magmacomputing 8194de7
new capture-group name syntax
magmacomputing 0edbe9c
test-cases
magmacomputing 8eafe0c
pre-discrete
magmacomputing a65a174
pre for-of
magmacomputing 7898eaf
pre alias-resolve
magmacomputing 03670a0
pre test-fails
magmacomputing 103fb5c
ready for review
magmacomputing 9e6beb6
migration phase2.1
magmacomputing 40545f8
ready for AliasMigration review
magmacomputing 3a5d834
Merge origin/main into release/D: resolve conflicts keeping release/D…
Copilot 6f5276c
Merge origin/main into release/D: resolve conflicts keeping release/D…
Copilot ec9e905
pre collapse alias
magmacomputing 1611506
align setEvents|setPeriods
magmacomputing c2fabf2
PR 1st review
magmacomputing 9d4b68d
PR 2nd review
magmacomputing 66e81ef
PR 3rd review
magmacomputing dd34f02
PR 4th review
magmacomputing File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| # Alias Migration: Phase 2 - Full Resolution Engine | ||
|
|
||
| This document outlines the remaining tasks to complete the migration from legacy alias management to the centralized `AliasEngine` architecture. The goal is to move all interpretation and mutation logic out of the Parser and into the Engine. | ||
|
|
||
| ## 1. Consolidate Resolution Context (The "Host" Object) | ||
| Currently, `discrete.parse.ts` manually constructs a "pseudo-Tempo" `host` object to pass into functional aliases. This logic should be standardized and moved to a helper. | ||
|
|
||
| - [x] Create a `getResolutionContext(state, dateTime)` helper in `support` or `AliasEngine`. | ||
| - [x] Ensure the context provides `add`, `subtract`, `with`, `set`, and time-unit accessors. | ||
| - [x] Remove the manual host construction from `discrete.parse.ts`. | ||
|
|
||
| ## 2. Hardened Clock Snapping | ||
| Aliases that resolve to a time-string (`hh:mm[:ss]`) currently have two different paths depending on whether they are static or functional. | ||
|
|
||
| - [x] **Standardize Paths**: Both static and functional aliases should trigger the "snap" path if they match `Match.clock`. | ||
| - [x] **Fix Precision Leak**: Ensure that snapping to a clock-time clears ALL sub-second components (ms, us, ns) from the anchor. | ||
| - [x] **Support High-Precision**: Update the snapping logic to support `hh:mm:ss.ffffff` patterns natively. | ||
| - [x] **Engine-Level Detection**: Move the `Match.clock` test into `AliasEngine.resolveAlias`. | ||
|
|
||
| ## 3. Rich Alias Results | ||
| Instead of returning a raw `string | number`, the `AliasEngine` should return a structured result object. | ||
|
|
||
| - [x] Define `AliasResult` interface: | ||
| ```typescript | ||
| interface AliasResult { | ||
| value: string; | ||
| key: string; // The original baseName (e.g., 'noon') | ||
| type: 'evt' | 'per'; | ||
| source: 'global' | 'local'; | ||
| isClock: boolean; // True if it matched Match.clock | ||
| isFunction: boolean; | ||
| } | ||
| ``` | ||
| - [x] Update `resolveAlias` to return this structure. | ||
|
|
||
| ## 4. Parser Cleanup | ||
| With the Engine handling the "what" and "how" of resolution, the Parser can focus on the "when". | ||
|
|
||
| - [x] Refactor `parseGroups` in `discrete.parse.ts` to consume the new `AliasResult`. | ||
| - [x] Remove manual string-splitting and mutation logic from the Parser. | ||
| - [x] Leverage the `source` metadata from the result instead of manually parsing regex group names (like `evt1_0`). | ||
| - [x] Extract `host` context construction to a helper. | ||
|
|
||
| ## 5. Lifecycle & Monitoring | ||
| - [x] Implement `AliasEngine.getVersion()` or similar to allow `Tempo` class to detect registry changes without deep-cloning. | ||
| - [x] Audit `tempo.class.ts` for any remaining direct access to `parse.event` or `parse.period`. | ||
|
|
||
| --- | ||
|
|
||
| > [!IMPORTANT] | ||
| > **Priority 1**: Hardening the clock-snapping logic and fixing the sub-second precision leak. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| tempo.class will invoke $setEvents() as part of the global, sandbox and instance setup. | ||
| events will be an array of [eventName, eventTarget] | ||
| where eventName is a plain string or a regex-like string (e.g. "xmas( )?eve"), and eventTarget is a string or Function that returns a string. | ||
| the eventTarget will name the date-string (e.g. "25-Dec") that should be interpreted when this eventName is detected by the parse-engine. | ||
|
|
||
| $setEvents will then run "events=ownEntries();" on the list of Events it has been provided (most likely just the Default, but could be more from global-discovery, localStorage, etc.). | ||
|
|
||
| If there are no events (which can happen), $setEvents exits... nothing to do. | ||
|
|
||
| If there are events, it should | ||
| check if there is an 'own' shape.aliasEngine, else allocate a 'new AliasEngine(...)" | ||
| the new aliasEngine should contain a reference to it's parent object... nothing for global, global for sandbox, global-or-sandbox for instances. | ||
| This hierarchy is important for Event resolution (see below). | ||
| each new aliasEngine should calculate it's own 'depth'... that is, | ||
| global => 0, | ||
| sandbox => 1+ (increasing for each sandbox created from another sandbox), | ||
| instance => 1 (if direct child of global) or => 2+ (if direct child of a sandbox) | ||
|
|
||
| $setEvents should then call aliasEngine.clear('event')... not sure if this is absolutely necessary, but couldn't hurt. | ||
|
|
||
| $setevents should then call "const groups = aliasEngine.registerEvents(events);" | ||
| to pass control to the shape's aliasEngine instance. | ||
|
|
||
| That instance will go through the 'events' array, and for each: | ||
| stash some related information into the Engine's instance so we can track | ||
| ### a sequential number to be allocated on an Event | ||
| ### the baseName | ||
| ### the eventTarget | ||
| ### the eventName (? not sure if this is needed ?) | ||
| a Set on the instance will track calc'd 'baseName' | ||
| it will also output a 'warn' if it detects that a baseName has already been used (whether in the current events-array or up the proto-chain). | ||
|
|
||
| Once the registration process is complete, it should return a regex-like string back to the caller in tempo.class. | ||
| The string will contain (from lowest to highest in the proto-chain) a calculated named-group regex source, with "(?<calc>eventTarget)" | ||
| the <calc> section will be "{depth}evt{index}" where depth is the aliasEngine's instance depth (0, 1, 2, etc.) and index is the sequential number that was assigned to an Event. | ||
| For example, passing in ['xmas','25-Dec'] from the global shape will have the registration return "((?<0evt1>xmas))" | ||
| For example, later passing in [bday; '20-May'] from an instance shape will have the registration return "((?<1evt1>bday)|(?<0evt1>xmas))" | ||
|
|
||
| When assembling the string to be returned (pipe-delimited named-group regex-source), the registration should: | ||
| ensure lower-depth regex-sources are returned prior to higher-depth | ||
| ensure that if a lower-depth is marked as a 'collision', then any higher-depth with that same baseName will be excluded | ||
|
|
||
| To use a Period as an example, assuming an instance wants to override a global definition of 'noon': | ||
| "new Tempo('noon': {period: {noon:'11:00'}}); | ||
| We would expect the depth for the Tempo-instance to be '1' (direct child of global shape) | ||
| We would expect the index to be '1' (first Period alias detected) | ||
| We would expect the registerEvents to return "((?<1per1>noon)|... the rest of the global Periods *except* where baseName is 'noon')" | ||
|
|
||
| When the calculated alias-string is returned to tempo.class, it will then update it's shadow the definition of the parent's snippets for Token.evt and Token.per. | ||
| tempo.class then calls setPatterns which will build the actual patterns (based on the current Layouts / Snippets) | ||
|
|
||
| ## Event Resolution | ||
| when the parsing engine detects a match against the patterns, and it finds a named-group with the pattern <nbr>evt<nbr> or <nbr>per<nbr>, then it knows it has an alias to de-reference. | ||
|
|
||
| It will find the aliasEngine that is associated with the current tempo-level being parse (global, sandbox, instance). | ||
|
|
||
| It will then invoke that aliasEngine's instance's method resolveEvent (or resolvePeriod) by passing in the named-group and the 'this' reference. | ||
|
|
||
| The aliasEngine will decode the 'depth' from the alias argument (the leading digits before the 'evt' or 'per' portion of the string), and travel up the proto-chain til it finds the correct instance that matches that depth. | ||
|
|
||
| The aliasEngine will then decode the 'index' from the alias argument (the trailing digit after the 'evt' or 'per' portion of the string) | ||
|
|
||
| That resolved instance will lookup its own registry of Aliases for the index of the eventTarget. | ||
|
|
||
| If the retrieved eventTarget is a string, it will return it to the parsing engine. | ||
| If the retrieved eventTarget is a Function, it will invoke the function (binding the 'this' context), and invoke a .toString() on the result before passing it back to the eventTarget;' | ||
|
|
||
| * what to do if the alias resolution is cyclic ? |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.