Skip to content

[refactor] Promote contextFromStoreNode to package scope; deduplicate Node→ContextNode reconstruction in write.go and summarize.go #202

Description

@radimsem

Summary

The 9-field reconstruction *store.Node*parser.ContextNode is copy-pasted in three places. A helper that already does exactly this — contextFromStoreNode — exists in forget.go but is file-private, so write.go and summarize.go re-inline the same field-by-field copy.

This has already caused fields to drift: summarize.go sets Temperature on its copy, but write.go's update branch does not, and contextFromStoreNode itself omits it. Any future field added to parser.ContextNode will need to be remembered in all three call sites — exactly the failure mode the helper was created to prevent.

Locations

  • pkg/mcp/tools/forget.go:123-135contextFromStoreNode (the existing helper)
  • pkg/mcp/tools/write.go:42-56 — update branch, full inline copy
  • pkg/mcp/tools/summarize.go:51-63 — full inline copy plus Temperature override

Category

refactor (duplication)

Impact

High — three call sites that must stay in sync; Temperature is already inconsistent across them.

Rationale

Same struct, same field order, same source fields. The only per-call-site difference is which fields are overridden afterward (label/content/tokenCount/contentHash in write; same plus Temperature in summarize). That is exactly the shape contextFromStoreNode already supports — start from the helper, then override.

Suggested change direction

  1. Move contextFromStoreNode out of forget.go into deps.go (the existing home for cross-handler helpers in this package).
  2. In write.go's update branch:
    node = contextFromStoreNode(existing)
    node.Label = label
    node.Content = payload
    node.TokenCount = tokenCount
    node.ContentHash = contentHash
  3. In summarize.go:
    node := contextFromStoreNode(existing)
    node.Label = "Summary: " + firstLine(summary, 70)
    node.Content = summary
    node.TokenCount = tokenCount
    node.ContentHash = contentid.ContentHash(summary)
    node.Temperature = &rebound
  4. Consider whether contextFromStoreNode itself should copy Temperature from the source node by default — likely yes for the write path (preserve existing temperature on update), with summarize overriding to the rebound value.

No new abstraction; just promote what already exists.


Filed by the nightly enhancement-scanner routine at commit e378bf7.

Metadata

Metadata

Assignees

No one assigned

    Labels

    auto-reviewFiled by the nightly enhancement-scanner routinerefactorCode refactoring / architecture cleanup

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions