Skip to content

Comments

docs: Minor tidying of docstrings and tool descriptions#228

Merged
IanMayo merged 4 commits intomainfrom
hotfix-more-tidying
Oct 7, 2025
Merged

docs: Minor tidying of docstrings and tool descriptions#228
IanMayo merged 4 commits intomainfrom
hotfix-more-tidying

Conversation

@IanMayo
Copy link
Member

@IanMayo IanMayo commented Oct 7, 2025

Summary

  • Updated docstrings in select_feature_start_time tool to clarify timestamp selection purpose
  • Refined tool directory description in init.py for better clarity

Test plan

  • Verify docstring changes improve code readability
  • Confirm no functional changes to tool behavior

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting

Comment on lines 546 to 555
const hasDebriefCommands = this._containsDebriefCommands(result);

if (hasDebriefCommands) {
const successMessage = `Tool "${commandName}" executed successfully and plot updated`;
vscode.window.showInformationMessage(successMessage);
// Tool updated plot - no notification needed on success
console.warn(`[DebriefActivityProvider] Tool "${commandName}" executed successfully and plot updated`);
} else {
const successMessage = `Tool "${commandName}" executed successfully`;
vscode.window.showInformationMessage(
successMessage,
'View Results',
'Copy Results'
).then(selection => {
if (selection === 'View Results') {
void this._showToolResults(commandName, result);
} else if (selection === 'Copy Results') {
void this._copyToolResults(result);
}
});
// Tool returned data - log success (no notification on success)
console.warn(`[DebriefActivityProvider] Tool "${commandName}" executed successfully`);
// Users can access results through the Activity view if needed
}

Choose a reason for hiding this comment

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

P1 Badge Removing success prompt hides access to tool results

The previous implementation showed an information message with View Results/Copy Results actions whenever a tool returned data. This change replaces that dialog with a console.warn, so _showToolResults and _copyToolResults are no longer invoked anywhere. As a result, users have no UI to open or copy the tool output after execution and the data is effectively inaccessible unless they inspect logs. This is a functional regression rather than a documentation tweak.

Useful? React with 👍 / 👎.

Comment on lines 511 to 520
const hasDebriefCommands = this._containsDebriefCommands(result);

if (hasDebriefCommands) {
// If tool returned commands, those were processed by GlobalController
const successMessage = `Tool "${commandName}" executed successfully and plot updated`;
vscode.window.showInformationMessage(successMessage);
// Tool updated plot - no notification needed on success
console.warn(`[DebriefOutlineProvider] Tool "${commandName}" executed successfully and plot updated`);
} else {
// Traditional result handling for tools that return data
const successMessage = `Tool "${commandName}" executed successfully`;
vscode.window.showInformationMessage(
successMessage,
'View Results',
'Copy Results'
).then(selection => {
if (selection === 'View Results') {
this._showToolResults(commandName, result);
} else if (selection === 'Copy Results') {
this._copyToolResults(result);
}
});
// Tool returned data - log success (no notification on success)
console.warn(`[DebriefOutlineProvider] Tool "${commandName}" executed successfully`);
// Users can access results through the Outline view if needed
}

Choose a reason for hiding this comment

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

P1 Badge Outline view loses ability to display tool results

Similar to the activity provider, the outline provider previously surfaced a notification with View Results/Copy Results when a tool returned data. The new code logs to the console instead, leaving _showToolResults and _copyToolResults unused and giving users no way to inspect or copy results from the outline view. This removes a core piece of functionality and likely wasn’t intended in a commit described as documentation cleanup.

Useful? React with 👍 / 👎.

@github-actions
Copy link

github-actions bot commented Oct 7, 2025

🔧 Tool Vault Build Complete

Commit: cc9ec0f
Artifact: Available in Actions tab
Preview App: https://toolvault-pr-228.fly.dev

The Tool Vault packager has been built and tested successfully.

@github-actions
Copy link

github-actions bot commented Oct 7, 2025

🚀 VS Code Extension PR Preview Deployed

Your VS Code extension PR preview has been successfully deployed to Fly.io!

🌐 Preview URL: https://pr-228-futuredebrief.fly.dev

📋 Details:

🔧 What's included:

  • VS Code (code-server) environment
  • Debrief extension pre-installed
  • Sample workspace with test files

💡 How to use:

  1. Click the preview URL above
  2. Open any .plot.json file to test the custom editor
  3. Use Ctrl+Shift+P to access extension commands
  4. Check the Explorer panel for the "Hello World" view

This preview will be automatically updated when you push new commits to this PR.

- Deleted PropertiesViewProvider.ts and TimeControllerProvider.ts files to streamline the codebase and eliminate unused components.
- Removed associated state management and event handling logic to consolidate state management within the GlobalController.
- Updated documentation to reflect the removal of these components and their functionalities.
@github-actions
Copy link

github-actions bot commented Oct 7, 2025

🔧 Tool Vault Build Complete

Commit: fe3e03a
Artifact: Available in Actions tab
Preview App: https://toolvault-pr-228.fly.dev

The Tool Vault packager has been built and tested successfully.

@IanMayo IanMayo merged commit 710dab6 into main Oct 7, 2025
4 checks passed
@IanMayo IanMayo deleted the hotfix-more-tidying branch October 7, 2025 13:53
@github-actions
Copy link

github-actions bot commented Oct 7, 2025

🧹 Tool Vault Cleanup Complete

The preview Fly.io app toolvault-pr-228 has been cleaned up after PR closure.

@github-actions
Copy link

github-actions bot commented Oct 7, 2025

🧹 VS Code Extension PR Preview Cleaned Up

Your PR preview environment has been automatically destroyed.

✅ Cleanup successful!

All associated resources have been removed from Fly.io to optimize costs.

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.

1 participant