-
Notifications
You must be signed in to change notification settings - Fork 0
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
chore: add plugin for logging context #39
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using this plugin is kind of manual job, it requires it to insert it into various locations, etc.
How about we add code to the middleware execution that will output evolution of context
through out the middleware processing.
In the first message it will show the initial context as it enters the pipeline. for each middleware step it will compare the context that entered the step vs context that exited the step and debug-log only the changes to the context.
Due to mutation of context object, we would either have to make deep copies of context or use JS Proxy. IMO deep copy sounds simpler.
@Q1w1N wdyt?
@mdjastrzebski Sound like a cool idea. However this plugin is supposed to be a quick and dirty way to just checkout the context in a specific way. It's basically "console.log("HERE")" version of debugging. |
a25ec45
to
d6f7b56
Compare
packages/core/src/plugins/logging.ts
Outdated
@@ -23,3 +25,27 @@ export const loggingPlugin: ApplicationPlugin = { | |||
} | |||
}, | |||
}; | |||
|
|||
export const contextLoggerBuilder = (fieldsToLog: (keyof RequestContext)[]): ApplicationPlugin => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make this optional, so that users skip the param to log the whole context
packages/core/src/plugins/logging.ts
Outdated
} | ||
|
||
if (Object.keys(toLog).length > 0) { | ||
console.log(inspect(toLog, false, null, true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logger
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modify logger to use inspect inside, detect if inspect is available.
@@ -28,7 +28,12 @@ const app = createApp({ | |||
chatModel, | |||
systemPrompt: SYSTEM_PROMPT, | |||
// Normalize messages coming from Slack AI apps | |||
plugins: [slackThreadNormalizerPlugin], | |||
plugins: [ | |||
contextLoggerBuilder([]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just put it once in the example
Summary
Added plugin for logging selected context properties
Test plan
Tested with example app