Skip to content
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

feat(auto-edits): clean auto-edits output channel #6547

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

hitesh-1997
Copy link
Contributor

@hitesh-1997 hitesh-1997 commented Jan 7, 2025

Context

Clean the output logger for auto-edits:

  1. Most of the debugging calls (to check where the auto-edits might break are enabled only if verbose debugging true.
  2. Large logs like prompt are shorted similar to autocomplete prompt logs.

Logs when verbose enabled for a single auto-edits request.
image

Logs without verbose enabled for a single auto-edits request.

  • Only response is logged
image

Test plan

  1. Trigger the auto-edits request
  2. See the output logs by toggling cody.debug.version flag

this.logger = new Logger(feature)
}

logDebugIfVerbose(filterLabel: string, text: string, ...args: unknown[]): void {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@valerybugakov do you think we should remove it and just logDebug them. In case any user faces some issue, they can share the logs with us (since atleast initial string will be present in logDebug)

Copy link
Member

Choose a reason for hiding this comment

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

Let's merge it as-is and iterate through follow-up PRs. Moving some of the logs under the verbose flag is the right move. In most cases, non-verbose logs should be sufficient, and enabling verbose logging should be straightforward for users.

Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

Thank you! I will merge it to help myself with local debugging.


constructor(feature: string) {
super(feature)
this.logger = new Logger(feature)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: extending the logger should be enough; there's no need to create an internal instance of the parent. We can still access its methods from this instance. I'll create a small patch for it.

@valerybugakov valerybugakov merged commit ba5d332 into main Jan 8, 2025
21 checks passed
@valerybugakov valerybugakov deleted the hitesh/clean-autoedits-output branch January 8, 2025 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants