Skip to content

drop 7 dead copilot.go methods, all superseded by a live sibling - #379

Merged
gerardrecinto merged 1 commit into
masterfrom
kill-7-dead-copilot-methods
Sep 19, 2026
Merged

gerardrecinto merged 1 commit into
masterfrom
kill-7-dead-copilot-methods

Conversation

@gerardrecinto

Copy link
Copy Markdown
Collaborator

7 more from the U1000 pile, each traced to a specific live replacement rather than just deleted on sight:

  • clearMRUCategory: superseded by clearMRUBySourceAndScope/clearMRUForTopicSwitch, both still live with more specific filtering criteria.
  • renderToolDefinitionContext: standalone orphan, no live caller anywhere in the registry-building code.
  • summarizeAskOutcomeMRU: thin wrapper combining buildAskOutcomeMRUItems + renderAskOutcomeMRUItems, both of which are called directly elsewhere without going through this wrapper.
  • resolvePersona: wrapper around resolvePersonaWithMetadata, which is itself still called directly at the one real call site.
  • getScriptToolsPrompt: legacy text-prompt builder listing available scripts for a text-based tool-calling prompt, superseded by listStoredScriptTools' native ai.ToolDefinition list. Same superseded era as BaselineReActEngine, removed earlier tonight in PR rip out unreachable BaselineReActEngine #368.
  • logThought: episodic-memory logging call. Its only caller was BaselineReActEngine.Run, already gone.
  • runScriptRaw: superseded by runScript/executeScriptSteps, both still live.

Also flagging something found while investigating this pile that I did NOT touch: active_memory.go's triggerSpaceAutoEnrichment has zero callers too, but unlike the above it's not superseded, it looks like a genuinely incomplete feature. KnowledgeBaseConfig.AllowAutoEnrichment is a real, settable field (wired through update_space_config), but nothing anywhere ever reads it except this one function, which nothing ever calls - no scheduler exists to wire it into without designing new Service-level periodic-task infrastructure. Left it alone and out of this PR since it's a bigger call than a same-cycle cleanup, noted for follow-up.

Build+vet+gofmt+test clean.

clearMRUCategory: superseded by clearMRUBySourceAndScope/
clearMRUForTopicSwitch, both still live with more specific filtering.
renderToolDefinitionContext: standalone orphan, no live caller anywhere.
summarizeAskOutcomeMRU: thin wrapper around buildAskOutcomeMRUItems +
renderAskOutcomeMRUItems, both of which are called directly elsewhere.
resolvePersona: wrapper around resolvePersonaWithMetadata, which is
itself still called directly.
getScriptToolsPrompt: legacy text-prompt builder for available scripts,
superseded by listStoredScriptTools' native ai.ToolDefinition list,
same era as BaselineReActEngine (removed earlier tonight, PR #368).
logThought: episodic-memory logging call, its only caller was
BaselineReActEngine.Run, already gone.
runScriptRaw: superseded by runScript/executeScriptSteps.
@github-actions

Copy link
Copy Markdown

Gemini PR Review

This pull request primarily involves the removal of several functions and associated imports from the CopilotAgent struct. This often indicates a significant refactoring or deprecation of features.

  • Code quality and readability

    • The removal of unused or refactored functions and imports generally improves code quality by reducing dead code and the overall complexity of the codebase. It makes the remaining code easier to understand and maintain.
  • Potential bugs and correctness issues

    • Functional Change: Thought Logging (logThought): The logThought function, which logged agent "episodes" (intents, AST, status, outcomes) to a knowledge base using embeddings, has been removed. This implies a significant change in how the agent's actions and thoughts are persisted or analyzed. If this logging was crucial for internal analytics, debugging, or a self-improvement feedback loop, its removal could lead to a loss of critical data or agent performance degradation over time. The removal of crypto/sha256 and the embed package import directly supports this, indicating the embedding and persistence mechanism for thoughts is gone.
    • Functional Change: Script Execution (runScriptRaw): The runScriptRaw function, which handled the detailed execution of script steps, including template resolution, per-step database overrides, and specific error handling (ContinueOnError, shouldShortCircuitScriptOnError), has been removed. This suggests that script execution logic has either been entirely refactored into runScript (which still exists) or another component, or these specific features (like per-step database overrides and granular error handling) are no longer supported. This is a substantial change to how scripts are processed and could introduce correctness issues if existing scripts relied on the removed features.
    • Functional Change: Persona Resolution (resolvePersona): The resolvePersona function, which returned a persona string, has been removed. resolvePersonaWithMetadata still exists. If resolvePersona was used without needing the metadata, its callers must now be updated, potentially with a different API or by extracting the persona from the metadata-rich function, which might change behavior if the metadata version had different side effects or guarantees.
    • Functional Change: MRU Categorization (clearMRUCategory): The function to clear MRU items by category has been removed. If there were use cases where a specific category of MRU items needed to be cleared (e.g., on feature disable, or context change), this functionality is now absent.
  • Missed edge cases

    • runScriptRaw Specifics: The runScriptRaw function had explicit handling for ContinueOnError and shouldShortCircuitScriptOnError for individual script steps, as well as a Database override. If these specific error handling behaviors or the ability to override the database per step are no longer present in the replacement logic, complex scripts that relied on these features may now behave incorrectly or fail in unexpected ways.
  • Performance considerations

    • The removal of logThought (which involved embedding computation and database write operations in a goroutine) could lead to a minor performance improvement by reducing background CPU, memory, and I/O load, especially if thought logging was frequent. The other removed functions are unlikely to have a noticeable performance impact.

@gerardrecinto
gerardrecinto merged commit cc8a88e into master Sep 19, 2026
21 checks passed
@gerardrecinto
gerardrecinto deleted the kill-7-dead-copilot-methods branch September 19, 2026 23:32
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