Skip to content

fix: OpenAI tool-parameter cache permanently retains every runtime's schema graphs - #1107

Open
sam-saffron-jarvis wants to merge 1 commit into
SamSaffron:mainfrom
sam-saffron-jarvis:feat/codereview-3d767e14
Open

fix: OpenAI tool-parameter cache permanently retains every runtime's schema graphs#1107
sam-saffron-jarvis wants to merge 1 commit into
SamSaffron:mainfrom
sam-saffron-jarvis:feat/codereview-3d767e14

Conversation

@sam-saffron-jarvis

Copy link
Copy Markdown
Contributor

What changed

  • Replaced the process-global, unbounded OpenAI parameter sync.Map with a mutex-protected identity cache capped at 4,096 entries.
  • Added O(1) FIFO eviction with a circular cursor, so evicted entries release their strong references to both the source schema graph and its lowered copy.
  • Kept sync.Once on each resident entry, preserving one lowering computation for concurrent callers that share a cached schema identity; returned parameter maps are still deep-copied.
  • Added a boundedness/eviction test that inserts more than the maximum number of distinct schema maps and verifies the oldest entry is gone, the newest is resident, and both cache bookkeeping structures remain capped.

Why this is high-value

OpenAI Responses and ChatGPT both use this cache for every tool-enabled request. Jarvis repeatedly creates short-lived web, Telegram, job, child-agent, and model-swap runtimes, and many built-in tools create fresh nested schema maps for each registry. The old global cache retained every source graph and lowered graph for the entire process lifetime, so heap retention and GC work grew with runtime churn.

The new fixed cap makes retained cache memory plateau instead of growing with every runtime. Hot schemas still avoid reparsing and normalization, while O(1) eviction avoids adding cache-size-dependent work once the cap is reached.

Validation

  • gofmt -w internal/llm/json_schema_lowering.go internal/llm/json_schema_model_test.go
  • go test ./internal/llm -run 'Test(OpenAIParameters|SanitizedResponsesParameters)' -count=1
  • go test -race ./internal/llm -run 'Test(OpenAIParameters|SanitizedResponsesParameters)' -count=1
  • go build ./...
  • git diff --check
  • go test ./... ran; all internal/llm tests passed. The repository-wide run was blocked by unrelated existing environment/main failures: two cmd skill-discovery tests saw the host's configured skill list instead of their temporary fixtures, and internal/serveui reported the freshly generated main-branch bundles slightly above existing gzip budgets. No failing test exercises the changed cache.

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