Uh oh!
There was an error while loading. Please reload this page.
Show opaque marker when deserializing custom classes and instances in o11y - #809
Merged
TooTallNate merged 8 commits intoJan 20, 2026
Conversation
🦋 Changeset detectedLatest commit: c889dd1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests💻 Local Development (2 failed)sveltekit-stable (2 failed):
🌍 Community Worlds (22 failed)mongodb (1 failed):
redis (1 failed):
starter (19 failed):
turso (1 failed):
Details by Category✅ ▲ Vercel Production
❌ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
❌ Some E2E test jobs failed:
Check the workflow run for details. |
This was referenced Jan 19, 2026
MemberAuthor
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Contributor
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the observability UI to display custom class instances that cannot be fully deserialized because their classes are not registered in the o11y context. The implementation adds opaque markers for both class instances and class references, making it easier to debug workflows that use custom classes.
Changes:
- Added
CLASS_INSTANCE_REF_TYPEmarker,ClassInstanceRefinterface, and type guards to represent unregistered class instances - Implemented helper functions (
extractClassName,serializedInstanceToRef,serializedClassToString) to convert serialized class data to display-friendly formats - Extended
streamPrintReviversto handleInstanceandClasstypes by converting them to opaque markers - Updated UI transformation logic to display class instances inline with a
__class__marker alongside their serialized data
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| packages/core/src/observability.ts | Adds ClassInstanceRef types, helper functions for class name extraction, and stream print revivers for Instance and Class types |
| packages/web-shared/src/sidebar/attribute-panel.tsx | Duplicates ClassInstanceRef types for client-side use and updates transformValueForDisplay to handle class instance refs inline with class marker |
| .changeset/forty-tables-lick.md | Documents the patch-level changes for both packages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| '__type' in value && | ||
| value.__type === CLASS_INSTANCE_REF_TYPE && | ||
| 'className' in value && | ||
| typeof value.className === 'string' |
There was a problem hiding this comment.
The type guard isClassInstanceRef validates className but not classId. For consistency with isStreamRef which validates all required fields, and to ensure type safety, the function should also validate that classId exists and is a string.
Suggested change
| typeofvalue.className==='string' | |
| typeofvalue.className==='string'&& | |
| 'classId'invalue&& | |
| typeofvalue.classId==='string' |
Comment on lines
+124
to
+128
| const extractClassName = (classId: string): string => { | ||
| if (!classId) return 'Unknown'; | ||
| const parts = classId.split('/'); | ||
| return parts[parts.length - 1] || classId; | ||
| }; |
There was a problem hiding this comment.
The extractClassName function returns 'Unknown' when classId is falsy, but an empty string is falsy in JavaScript. When classId is an empty string, the function would return 'Unknown' without trying to split it. However, if the split produces an empty array or the last part is an empty string, it would fall back to returning the original classId. Consider handling the empty string case explicitly before the split operation to ensure consistent behavior.
Comment on lines
+244
to
+249
| return { | ||
| __class__: v.className, | ||
| ...(typeof v.data === 'object' && v.data !== null | ||
| ? v.data | ||
| : { value: v.data }), | ||
| }; |
There was a problem hiding this comment.
The spread operator on line 247 could potentially overwrite the class property if v.data is an object that contains a class key. This would result in the class name information being lost. Consider using a different marker name that's less likely to collide with actual object properties, or ensure the class property cannot be overwritten by spreading v.data after it instead of before.
Suggested change
| return{ | |
| __class__: v.className, | |
| ...(typeofv.data==='object'&&v.data!==null | |
| ? v.data | |
| : {value: v.data}), | |
| }; | |
| returntypeofv.data==='object'&&v.data!==null | |
| ? { | |
| ...(v.dataasRecord<string,unknown>), | |
| __class__: v.className, | |
| } | |
| : { | |
| value: v.data, | |
| __class__: v.className, | |
| }; |
Comment on lines
+244
to
+248
| return { | ||
| __class__: v.className, | ||
| ...(typeof v.data === 'object' && v.data !== null | ||
| ? v.data | ||
| : { value: v.data }), |
There was a problem hiding this comment.
The v.data object is spread directly without being transformed recursively. If v.data contains nested StreamRef or ClassInstanceRef objects, they won't be properly handled. The nested data should be recursively transformed before spreading to ensure all special references are properly converted.
Suggested change
| return{ | |
| __class__: v.className, | |
| ...(typeofv.data==='object'&&v.data!==null | |
| ? v.data | |
| : {value: v.data}), | |
| consttransformedData= | |
| typeofv.data==='object'&&v.data!==null | |
| ? transform(v.data) | |
| : transform(v.data); | |
| return{ | |
| __class__: v.className, | |
| ...(typeofv.data==='object'&&v.data!==null | |
| ? (transformedDataasRecord<string,unknown>) | |
| : {value: transformedData}), |
Comment on lines
+151
to
157
| const serializedClassToString = (value: { classId: string }): string => { | ||
| const className = extractClassName(value.classId); | ||
| return `<class:${className}>`; | ||
| }; | ||
| /** | ||
| * This is an extra reviver for devalue that takes any streams that would be converted, |
There was a problem hiding this comment.
The serializedClassToString function doesn't validate that the input value has the expected structure. If value.classId is undefined or not a string, extractClassName will handle it, but it would be clearer to add explicit validation to ensure the function is called with the expected shape of data.
Suggested change
| constserializedClassToString=(value: {classId: string}): string=>{ | |
| constclassName=extractClassName(value.classId); | |
| return`<class:${className}>`; | |
| }; | |
| /** | |
| *Thisisanextrareviverfordevaluethattakesanystreamsthatwouldbeconverted, | |
| *Convertaserializedclassreferencetoa string representation. | |
| *ThisisusedforClasstype(theconstructorreferenceitself,notaninstance). | |
| */ | |
| constserializedClassToString=(value: unknown): string=>{ | |
| if( | |
| !value|| | |
| typeofvalue!=='object'|| | |
| !('classId'invalue)|| | |
| typeof(valueas{classId: unknown}).classId!=='string' | |
| ){ | |
| return'<class:Unknown>'; | |
| } | |
| constclassId=(valueas{classId: string}).classId; | |
| constclassName=extractClassName(classId); | |
| return`<class:${className}>`; | |
| }; |
Comment on lines
+139
to
+143
| return { | ||
| __type: CLASS_INSTANCE_REF_TYPE, | ||
| className: extractClassName(value.classId), | ||
| classId: value.classId, | ||
| data: value.data, |
There was a problem hiding this comment.
The serializedInstanceToRef function doesn't validate that the input value has the expected structure. While the type annotation suggests the expected shape, adding runtime validation would make the function more robust, especially since it's called from the streamPrintRevivers which receives values of type any.
Suggested change
| return{ | |
| __type: CLASS_INSTANCE_REF_TYPE, | |
| className: extractClassName(value.classId), | |
| classId: value.classId, | |
| data: value.data, | |
| // Runtime validation because this function is ultimately called with values of type `any`. | |
| constisObject=value!==null&&typeofvalue==='object'; | |
| constclassId=isObject&&typeof(valueasany).classId==='string' | |
| ? (valueasany).classId | |
| : ''; | |
| constdata=isObject ? (valueasany).data : value; | |
| return{ | |
| __type: CLASS_INSTANCE_REF_TYPE, | |
| className: extractClassName(classId), | |
| classId, | |
| data, |
Comment on lines
+120
to
+154
| /** | ||
| * Extract the class name from a classId. | ||
| * The classId format is typically "path/to/file/ClassName" so we extract the last segment. | ||
| */ | ||
| const extractClassName = (classId: string): string => { | ||
| if (!classId) return 'Unknown'; | ||
| const parts = classId.split('/'); | ||
| return parts[parts.length - 1] || classId; | ||
| }; | ||
| /** | ||
| * Convert a serialized class instance to a ClassInstanceRef for o11y display. | ||
| * This allows viewing custom class instances in the UI without needing | ||
| * the class to be registered for deserialization. | ||
| */ | ||
| const serializedInstanceToRef = (value: { | ||
| classId: string; | ||
| data: unknown; | ||
| }): ClassInstanceRef => { | ||
| return { | ||
| __type: CLASS_INSTANCE_REF_TYPE, | ||
| className: extractClassName(value.classId), | ||
| classId: value.classId, | ||
| data: value.data, | ||
| }; | ||
| }; | ||
| /** | ||
| * Convert a serialized class reference to a string representation. | ||
| * This is used for Class type (the constructor reference itself, not an instance). | ||
| */ | ||
| const serializedClassToString = (value: { classId: string }): string => { | ||
| const className = extractClassName(value.classId); | ||
| return `<class:${className}>`; | ||
| }; |
There was a problem hiding this comment.
The new helper functions extractClassName, serializedInstanceToRef, and serializedClassToString lack test coverage. Given that other utility functions in the codebase have comprehensive test files (e.g., serialization.test.ts, parse-name.test.ts), consider adding tests to verify the behavior of these functions, especially edge cases like empty classId, classIds with different path separators, and handling of various data types.
| '__type' in value && | ||
| value.__type === CLASS_INSTANCE_REF_TYPE && | ||
| 'className' in value && | ||
| typeof value.className === 'string' |
There was a problem hiding this comment.
The type guard isClassInstanceRef validates className but not classId. For consistency with isStreamRef which validates all required fields, and to ensure type safety, the function should also validate that classId exists and is a string. This would prevent runtime errors if a malformed ClassInstanceRef object is encountered.
Suggested change
| typeofvalue.className==='string' | |
| typeofvalue.className==='string'&& | |
| 'classId'invalue&& | |
| typeofvalue.classId==='string' |
TooTallNateforce-pushed
the
01-19-show_opaque_marker_when_deserializing_custom_classes_and_instances_in_o11y
branch
from
January 19, 2026 22:26
127cfec to
c46327fCompareTooTallNateforce-pushed
the
01-09-add_support_for_custom_class_instance_serialization
branch
from
January 19, 2026 22:26
baea85a to
4216347Compare| value: unknown | ||
| ): { json: string; streamRefs: Map<string, StreamRef> } => { | ||
| ): { | ||
| json: string; |
Contributor
There was a problem hiding this comment.
ClassInstanceRef transformation allows properties in v.data to overwrite the class marker, and doesn't recursively transform nested ClassInstanceRef objects within v.data
| * Used in o11y when a custom class instance is encountered but the class is not | ||
| * registered for deserialization. | ||
| */ | ||
| interface ClassInstanceRef { |
Contributor
There was a problem hiding this comment.
Type guard isClassInstanceRef doesn't validate classId and data properties required by ClassInstanceRef interface, leading to type unsafety
| '__type' in value && | ||
| value.__type === CLASS_INSTANCE_REF_TYPE && | ||
| 'className' in value && | ||
| typeof value.className === 'string' |
Contributor
There was a problem hiding this comment.
The isClassInstanceRef type guard doesn't validate classId and data properties required by the ClassInstanceRef interface
TooTallNateforce-pushed
the
01-19-show_opaque_marker_when_deserializing_custom_classes_and_instances_in_o11y
branch
from
January 19, 2026 23:14
a14fe6d to
04b36f5CompareContributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Stream Benchmarks(includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro | Express SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
VaguelySerious
approved these changes
Jan 20, 2026
| className="inline-flex flex-col rounded text-[11px] font-mono my-1" | ||
| style={{ | ||
| backgroundColor: colors.body, | ||
| border: `1px solid ${colors.header}`, |
There was a problem hiding this comment.
This leaves some weird extra white pixel borders
Suggested change
| border: `1px solid ${colors.header}`, |
There was a problem hiding this comment.
Actually I tested too early, this seems to already be resolved
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Peter Wielander <mittgfu@gmail.com> Signed-off-by: Nathan Rajlich <n@n8.io>
Merged
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Web example:
dark mode
light mode
CLI example:
Added support for displaying custom class instances in the observability UI by showing opaque markers when deserializing custom classes.
What changed?
CLASS_INSTANCE_REF_TYPEmarker and related interfaces to represent serialized class instances that cannot be fully deserialized__class__marker alongside the serialized dataHow to test?
<class:ClassName>in the UIWhy make this change?
Previously, custom class instances couldn't be properly displayed in the observability UI because they weren't registered for deserialization in the o11y context. This change improves the developer experience by providing meaningful representations of custom classes and instances, making it easier to debug and understand workflow execution without requiring class registration.