Skip to content

Clean up schema discovery and add Principal in TS SDK#2608

Merged
mschuwalow merged 82 commits intomainfrom
feature_2551
Jan 23, 2026
Merged

Clean up schema discovery and add Principal in TS SDK#2608
mschuwalow merged 82 commits intomainfrom
feature_2551

Conversation

@afsalthaj
Copy link
Copy Markdown
Contributor

@afsalthaj afsalthaj commented Jan 19, 2026

Fixes #2551
Fixes #2613

While adding support of principal, i made a lot of cleanups. While certain parts was just code moving aaround, certain other parts mainy schema module was heavily refactored. And then it was much easier to add support for Principal.

Also note that, I couldn't do all cleaning (so certain code that was moved around might still look to be something that can be improved). Allow me to do follow ups for that, because it took a lot of time for me to figure out how i broke integration tests: https://github.com/golemcloud/golem/actions?query=is%3Afailure+branch%3Afeature_2551 , and finally it is green.

Apart from the whole schema module being refactored, the following specific clean ups was done.

  • There was TypeDetails, TypeInfoInternal which are internal representation of a type (a method parameter type, constructor parameter type or the return type). Removing TypeDetails just led to a few changes. We need only 1 internal type.
  • TypeInfoInternal now holds WitType as well, which makes it easy to find the ElementSchema, or DataSchema for any multimodal type or generally any return type - makes things easy to comprehend.
  • Finally, start a schema package and separate out functions that's dedicated to constructor, method input and method output.
  • Move http module inside internal.
  • This leads to simply adding 1 more variant to just TypeInfoInternal called Principal and work backwards

@afsalthaj
Copy link
Copy Markdown
Contributor Author

afsalthaj commented Jan 21, 2026

There are other bugs which is already fixed. The main one was #2613 . the last two things spotted is:

  1. For multimodal, more than 1 parameter can exist as far as the rest is Principal.
  2. Handle constructor parameters or method parameters with ? during auto injection

}
}

return vars;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this is showing, all files in http should have been a file renaming.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was getting overly complicated, hence refactored the code along with re-organisation

@afsalthaj afsalthaj marked this pull request as ready for review January 22, 2026 05:46
@afsalthaj
Copy link
Copy Markdown
Contributor Author

1 more integration test failure to fix which is

    async funOptionalQMark(param1: string, param2?: number, param3?: string) {
        return self.barAgent(param1, param2, param3)
    }

After auto injection, the RPC is failing for these question mark parameters.

switch (elem.tag) {
case 'unstructured-text':
const parameterDetail = paramTypes[idx];
paramTypes.map((parameterDetail) => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we mapped data value elements before. This time we are mapping parameter types and then lookup data value elements. This is an important change to get errors

@afsalthaj
Copy link
Copy Markdown
Contributor Author

afsalthaj commented Jan 23, 2026

While adding support of principal, i made a lot of cleanups. While certain parts was just code moving aaround, certain other parts mainy schema module was heavily refactored. And then it was much easier to add support for Principal.

Also note that, I couldn't do all cleaning (certain bits within schema module too, and some code that you see was just code moved around and might still look to be something that can be improved). Allow me to do follow ups for that, because it took a lot of time for me to figure out how i broke integration tests - to finally - make it green. https://github.com/golemcloud/golem/actions?query=is%3Afailure+branch%3Afeature_2551 , and finally it is green.

// Deserializing rpc result doesn't require principal context,
// In this case typeInfoInternal is 'unstructured-text', so Principal type cannot appear here
],
{ tag: 'anonymous' },
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do something about it - to be more clearer. but I have already done a lot of cleanups elsewhere to avoid these kind of situations to add Principal feature, and can clean ups in follow up PR.

@mschuwalow mschuwalow merged commit 8cc2dc1 into main Jan 23, 2026
49 of 50 checks passed
@mschuwalow mschuwalow deleted the feature_2551 branch January 23, 2026 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants