feat: improvements #169

gentlementlegen · 2024-10-25T03:19:35Z

Relates to #138

Description of Changes

This pull request introduces the following changes:

package.json:
- Corrected formatting in the "exports" section.
- Added @octokit/plugin-paginate-graphql as a dependency.
src/sdk/actions.ts:
- Introduced a new constant HEADER_NAME with the value "Ubiquity".
- Renamed postComment function to postErrorComment for clarity.
- Enhanced the postComment function logging to accurately reflect posting error comments.
- Added a new function createStructuredMetadata to format log metadata in a structured manner within the comments posted to GitHub.

Introduce postComment function to embed structured metadata in comments.

Added export for Manifest type to facilitate type usage in SDK implementations.

Introduced the paginateGraphQL plugin to enhance API request handling.

gentlementlegen · 2024-10-25T03:45:19Z

@UbiquityOS can you write the summary of changes made on that pull-request?

ubiquity-os-beta · 2024-10-25T03:46:18Z

Summary of Changes in Pull Request #169

This pull request introduces several important updates and improvements to the ubiquity-os/ubiquity-os-kernel project:

package.json Enhancements
- Formatting Corrections: Improved the formatting in the exports section for better clarity and consistency.
- New Dependency: Added octokit/plugin-paginate-graphql to the dependencies to enhance GitHub API interactions.
src/sdk/actions.ts Updates
- New Constant: Introduced a new constant headerName with the value ubiquity to standardize header naming.
- Function Renaming: Renamed the postComment function to postErrorComment to better reflect its purpose of handling error comments.
- Enhanced Logging: Improved logging within the postErrorComment function to provide more accurate and detailed information when posting error comments.
- Structured Metadata Function: Added a new function createStructuredMetadata to format log metadata in a structured manner, ensuring consistency in comments posted to GitHub.
Configuration File Renaming
- Updated Configurations: Renamed configuration files from .ubiquibot-config.yml to .ubiquity-os.config.yml to align with the new naming conventions.
- Logger Package Migration: Migrated from @ubiquity-dao/ubiquibot-logger to @ubiquity-dao/ubiquity-os-logger to maintain consistency across the project.
Continuous Integration (CI) Adjustments
- CI Fixes: Addressed CI failures by migrating the logger package and updating relevant configurations.
- Ensured CI Stability: Verified that all CI processes pass successfully after the migration, ensuring reliable build and deployment pipelines.
Additional Improvements
- Issue Linking: Implemented stricter linking of issue numbers (e.g., resolves #1234) to ensure proper association between pull requests and issues.
- Code Clean-Up: Performed various code clean-ups and optimizations for better maintainability and readability.

This pull request effectively addresses Issue #138 by implementing these changes, resulting in a more structured, maintainable, and reliable codebase. The updates enhance the project's configuration management, improve logging mechanisms, and ensure seamless integration with GitHub's API and CI pipelines.

References:

gentlementlegen · 2024-10-25T03:48:41Z

@whilefoo @Keyrxng if you can have a look, can't assign you, thanks!

Updated @sinclair/typebox dependency version for compatibility.

Introduce try-catch blocks for schema decoding to handle errors gracefully.

Keyrxng

A test for this fn would be nice and would resolve my concerns re: new Error, looks good otherwise.

src/sdk/actions.ts

Consolidated variable initialization and improved error stack handling.

Separated postComment from actions and server files for better modularity.

Ensure proper formatting by adding carriage returns around serialized metadata.

# Conflicts: # src/sdk/server.ts

Change from `payload.settings` to `inputs.settings` for accuracy in error logs.

Changed the comment body in `comment.ts` to use `message.logMessage.diff`.

Deleted revert-pr-108.txt and modified plugin-configuration.ts to permit additional properties.

src/sdk/actions.ts

Validate input schema and log errors if the body is invalid.

Improve readability by storing honoEnv result to a variable before use.

Keyrxng · 2024-11-04T15:47:37Z

src/sdk/comment.ts

+ */
+export async function postComment(context: Context, message: LogReturn) {
+  if ("issue" in context.payload && context.payload.repository?.owner?.login) {
+    const metadata = createStructuredMetadata(message.metadata?.name, message);


message.metadata?.name will require we update the logger. I implemented pluginName in this logger PR, is that what name represents here?

No, name represents className not pluginName sorry, I'm confused what className represents now is it the header by which to parse the structuredMetadata e.g ask-llm-response

Keyrxng · 2024-11-04T15:48:38Z

throw new Error() prints Error twice, not sure if that is something that should be cleaned up
Some string sanitation is affecting the metadata log when throwing new Error
No stack trace in either of them (this is likely a logger issue)

Is this how consumers are expected to build plugins now?

Keyrxng · 2024-11-04T15:57:26Z

src/sdk/comment.ts

+import { LogReturn } from "@ubiquity-os/ubiquity-os-logger";
+import { sanitizeMetadata } from "./util";
+
+const HEADER_NAME = "Ubiquity";


Confused on the purpose of this but if the full header is how we'll search for and parse the structured metadata then we can pull the org via the payload

Keyrxng · 2024-11-04T16:00:01Z

src/sdk/comment.ts

+ * Posts a comment on a GitHub issue if the issue exists in the context payload, embedding structured metadata to it.
+ */
+export async function postComment(context: Context, message: LogReturn) {
+  if ("issue" in context.payload && context.payload.repository?.owner?.login) {


Should we add if("pull_request" in ...) as well and use pull_request.number?

I'm not 100% if payloads with "pull_request" in it always have "issue" too

Keyrxng · 2024-11-04T16:13:39Z

I spent like 40mins building complex types to satisfy typebox between the plugin-template and this sdk and turns out typebox versions were not the same lmao, 20 mins to figure out how to use the sdk right.

I haven't QA'd the action side of things (don't think that's necessary given the changes)

Invalid schemas are thrown correctly:

Keyrxng · 2024-11-04T16:25:35Z

src/sdk/comment.ts

+
+  if (logMessage?.type === "fatal") {
+    // if the log message is fatal, then we want to show the metadata
+    metadataSerialized = [metadataSerializedVisible, metadataSerializedHidden].join("\n");


Is this how it should be displayed? Why don't we just replace <!-- --!> with backticks and show the structured data with the header, is there a reason?

I think because it allows us to search for the metadata

I thought search would be based on the components of the header not including the HTML comment syntax <!-- at the start of the header. Bit harder to parse I guess without it, my point was duplicating the metadata but it's not a real problem

It's very unlikely we'll ever search by caller or by revision I think, what about you guys?

If we used just the headers we could do something like this UbiquityOS - <orgName> - <pluginName>. First item being the name of the bot, just to make the query a bit more specific. If className === LogLevel then that gives us a type of filter for search if we include it.

Is there a spec for search/parse implementation yet?

when we search we need to search for the comment syntax otherwise it can match other things, so we need to search for <-- UbiquityOS ... <metadata> ... -->

I can't resolve this but think it can be if double metadata being posted is not a problem

Keyrxng · 2024-11-04T16:55:52Z

src/sdk/comment.ts

+  const jsonPretty = sanitizeMetadata(metadata);
+  const stack = logReturn.metadata?.stack;
+  const stackLine = (Array.isArray(stack) ? stack.join("\n") : stack)?.split("\n")[2] ?? "";
+  const caller = stackLine.match(/at (\S+)/)?.[1] ?? "";


The QA images I grabbed show that caller is always undefined no matter if it's new Error or throw logger

You can take caller from LogReturn it should already have it and so should Error too.

Personally I don't see the value in sticking the invoking fn name in the header used for search, but maybe that's short sighted of me.

All of that code was some copy paste of some function you implemented in another project I think, forgot which one, will update that.

It's implemented in command-ask but I only added it last week and took it from this PR. It does look familiar tho I can agree with that, akin to the logger internals I thought at first. Possible I wrote it somewhere else but I can't recall

https://github.com/ubiquity-os-marketplace/command-ask/blame/eb5487d4879e48757ff758ed3fa90728469edcc1/src/handlers/comment-created-callback.ts#L63

It's from Ubiquibot V1 actually as far back as that

Replaced ctx.req.json() with body in input decoding to ensure valid input handling.

gentlementlegen · 2024-11-05T15:20:10Z

Since most of the requested changes are about the metadata, we need a PR in the logger to be merged, then the changes will be properly carried out within #180.

Keyrxng · 2024-11-05T20:59:31Z

Since most of the requested changes are about the metadata, we need a PR in the logger to be merged, then the changes will be properly carried out within #180.

Everything handled within the SDK via gentlementlegen#1 so no need to update the logger right now maybe not at all

gentlementlegen · 2024-11-06T04:59:18Z

I'll merge this so I can get it deployed on NPM. In the meantime I reviewed your PR @Keyrxng that would solve the related issue. If there is any post merge comment I'll take care of it as well.

gentlementlegen added 3 commits October 25, 2024 11:52

fix(sdk): add postComment function for structured metadata

b87f4ca

Introduce postComment function to embed structured metadata in comments.

fix: export Manifest type from SDK

2b090cd

Added export for Manifest type to facilitate type usage in SDK implementations.

fix: add paginateGraphQL plugin to customOctokit

a3e1202

Introduced the paginateGraphQL plugin to enhance API request handling.

gentlementlegen marked this pull request as ready for review October 25, 2024 03:48

gentlementlegen added 3 commits October 25, 2024 13:00

chore(deps): update @sinclair/typebox to ^0.33.17

c9b92bc

Updated @sinclair/typebox dependency version for compatibility.

fix: add error handling to schema decoding

7b15aa1

Introduce try-catch blocks for schema decoding to handle errors gracefully.

chore: fix compile error

9aa4aec

Keyrxng reviewed Oct 25, 2024

View reviewed changes

src/sdk/actions.ts Outdated Show resolved Hide resolved

src/sdk/actions.ts Outdated Show resolved Hide resolved

whilefoo reviewed Oct 25, 2024

View reviewed changes

src/sdk/actions.ts Show resolved Hide resolved

src/sdk/actions.ts Outdated Show resolved Hide resolved

Keyrxng mentioned this pull request Oct 25, 2024

feat: dynamic ground truths ubiquity-os-marketplace/command-ask#14

Merged

Keyrxng reviewed Oct 25, 2024

View reviewed changes

src/sdk/actions.ts Outdated Show resolved Hide resolved

Keyrxng reviewed Oct 25, 2024

View reviewed changes

src/sdk/actions.ts Outdated Show resolved Hide resolved

ubiquity-os deleted a comment from Keyrxng Oct 26, 2024

gentlementlegen added 3 commits October 28, 2024 11:56

refactor: simplify createStructuredMetadata function

99ef8ab

Consolidated variable initialization and improved error stack handling.

refactor: move postComment to its own file

49e4220

Separated postComment from actions and server files for better modularity.

chore: add carriage returns to serialized metadata

1d3ee4c

Ensure proper formatting by adding carriage returns around serialized metadata.

gentlementlegen mentioned this pull request Oct 28, 2024

Properly set the metadata value of LogReturn ubiquity-os/ubiquity-os-logger#46

Open

gentlementlegen requested review from whilefoo, Keyrxng and 0x4007 October 28, 2024 03:05

gentlementlegen and others added 4 commits October 28, 2024 13:01

Merge branch 'development' into feat/improvements

d2746d1

# Conflicts: # src/sdk/server.ts

fix: correct variable used in error logging

37ca024

Change from `payload.settings` to `inputs.settings` for accuracy in error logs.

fix: update comment body to use diff instead of raw log

1ccbac3

Changed the comment body in `comment.ts` to use `message.logMessage.diff`.

chore(config): allow additional properties in plugin configuration

2cbf1ed

Deleted revert-pr-108.txt and modified plugin-configuration.ts to permit additional properties.

whilefoo reviewed Oct 29, 2024

View reviewed changes

src/sdk/actions.ts Show resolved Hide resolved

gentlementlegen mentioned this pull request Oct 30, 2024

Improve Cloudflare Worker postComment metadata #180

Open

chore(validation): add input schema validation for request body

1aa8f0c

Validate input schema and log errors if the body is invalid.

refactor: extract honoEnv call to a variable

f30dbb5

Improve readability by storing honoEnv result to a variable before use.

This was referenced Nov 1, 2024

feat: migration to SDK ubiquity-os-marketplace/text-conversation-rewards#170

Merged

feat/label-gating ubiquity-os-marketplace/daemon-pricing#42

Merged

gentlementlegen requested a review from whilefoo November 3, 2024 04:45

Keyrxng reviewed Nov 4, 2024

View reviewed changes

fix: resolve input decoding issue in server.ts

fe99961

Replaced ctx.req.json() with body in input decoding to ensure valid input handling.

gentlementlegen merged commit 393ca5d into ubiquity-os:development Nov 6, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: improvements #169

feat: improvements #169

gentlementlegen commented Oct 25, 2024 •

edited

Loading

gentlementlegen commented Oct 25, 2024

ubiquity-os-beta bot commented Oct 25, 2024

gentlementlegen commented Oct 25, 2024

Keyrxng left a comment

Keyrxng Nov 4, 2024 •

edited

Loading

Keyrxng commented Nov 4, 2024 •

edited

Loading

Keyrxng Nov 4, 2024

Keyrxng Nov 4, 2024

Keyrxng commented Nov 4, 2024

Keyrxng Nov 4, 2024 •

edited

Loading

whilefoo Nov 4, 2024

Keyrxng Nov 4, 2024 •

edited

Loading

whilefoo Nov 4, 2024

Keyrxng Nov 4, 2024

Keyrxng Nov 4, 2024 •

edited

Loading

gentlementlegen Nov 5, 2024

Keyrxng Nov 5, 2024 •

edited

Loading

Keyrxng Nov 5, 2024 •

edited

Loading

gentlementlegen commented Nov 5, 2024

Keyrxng commented Nov 5, 2024

gentlementlegen commented Nov 6, 2024

feat: improvements #169

feat: improvements #169

Conversation

gentlementlegen commented Oct 25, 2024 • edited Loading

Description of Changes

gentlementlegen commented Oct 25, 2024

ubiquity-os-beta bot commented Oct 25, 2024

gentlementlegen commented Oct 25, 2024

Keyrxng left a comment

Choose a reason for hiding this comment

Keyrxng Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

Keyrxng commented Nov 4, 2024 • edited Loading

Keyrxng Nov 4, 2024

Choose a reason for hiding this comment

Keyrxng Nov 4, 2024

Choose a reason for hiding this comment

Keyrxng commented Nov 4, 2024

Keyrxng Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

whilefoo Nov 4, 2024

Choose a reason for hiding this comment

Keyrxng Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

whilefoo Nov 4, 2024

Choose a reason for hiding this comment

Keyrxng Nov 4, 2024

Choose a reason for hiding this comment

Keyrxng Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

gentlementlegen Nov 5, 2024

Choose a reason for hiding this comment

Keyrxng Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Keyrxng Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

gentlementlegen commented Nov 5, 2024

Keyrxng commented Nov 5, 2024

gentlementlegen commented Nov 6, 2024

gentlementlegen commented Oct 25, 2024 •

edited

Loading

Keyrxng Nov 4, 2024 •

edited

Loading

Keyrxng commented Nov 4, 2024 •

edited

Loading

Keyrxng Nov 4, 2024 •

edited

Loading

Keyrxng Nov 4, 2024 •

edited

Loading

Keyrxng Nov 4, 2024 •

edited

Loading

Keyrxng Nov 5, 2024 •

edited

Loading

Keyrxng Nov 5, 2024 •

edited

Loading