You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat: Add versioned, cross-language Selene API models - #212
result uses Pydantic's coercing boolean type, so parse_trace accepts schema-invalid JSON such as "result": 1 or "result": "false" and normalizes it, while the JSON Schema and Rust/TypeScript bindings require an actual boolean. Use the existing strict boolean alias here so all bindings validate the same wire values.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
TypeScript parsing and constructor composition can fail unexpectedly, while Rust rejects Base64 accepted by the canonical schemas.
Review effort: Balanced Findings: None
Files not reviewed (1)
pnpm-lock.yaml: Generated file
Previously missed (3)
In code that hasn't changed since last review
Align Base64 pad-bit validation across implementations
selene-trace/rust/src/trace.rs:234
The checked-in Base64 patterns and the TypeScript/Python bindings accept encodings with non-zero unused pad bits (for example, "Zh==" decodes to b"f"), but base64's default URL_SAFE engines reject them. Consequently, a document valid under either published schema can fail only in Rust. Either configure this decoder to match the accepted grammar or enforce canonical pad bits consistently in both schemas and the other bindings.
Guard BigInt conversion after regex validation fails
selene-trace/typescript/src/trace.ts:28
Zod continues into this refinement after a preceding regex check has marked the parse invalid, so inputs such as "abc" call BigInt("abc") and throw SyntaxError. As a result, UInt64DecimalStringSchema.safeParse and safeParseTrace can throw instead of returning a failed parse. Guard the conversion so every invalid string remains a Zod validation failure.
Allow constructors to accept normalized trace outputs
selene-trace/typescript/src/trace.ts:260
These constructors are not composable with their own normalized outputs. createTraceData converts an opaque tag from a decimal string to bigint, but createTraces reparses it through an input schema that accepts only strings, so createTraces([createTraceData(opaqueEvents)]) throws; similarly, the README's createTraceData(document.events) does not type-check when document is parsed. The constructor layer should accept normalized EventRecord/TraceData values as well as wire inputs without weakening the wire parsers.
The reason will be displayed to describe this comment to others. Learn more.
IIRC you can have a step invoke python directly by using shell: python, then make the run contents just the python script. Might be cleaner than bash/heredoc.
OTOH there may be a good case for turning this into a file and having it invokable locally as part of a git check.
The reason will be displayed to describe this comment to others. Learn more.
I now check that the package versions match using a separate script on every commit. The script prints the shared version on success and I use it here in a normal shell to compare to the release version.
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
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.
Introduces the selene-trace workspace for versioned, serializable Selene API models.
TraceandTraces, which both carry a required schema version.OpaquePayload.tagis encoded as canonical hexadecimal strings in JSON for cross-language precision safety.Before the first release, configure: