feat(ai-builder): Surface thread provenance on Instance AI traces (no-changelog) - #36909
Open
JoseBra wants to merge 2 commits into
Open
feat(ai-builder): Surface thread provenance on Instance AI traces (no-changelog)#36909JoseBra wants to merge 2 commits into
JoseBra wants to merge 2 commits into
Conversation
…-changelog) A thread already records where it was opened from — `source` (required, from INSTANCE_AI_THREAD_SOURCES) plus the opener's own `sourceContext` bag, both persisted in the thread's metadata by `ensureThread`. Neither ever reached the trace, so a LangSmith project cannot answer "which entry point produced this run?" — and, for the offline eval harness, cannot tell one build from the hundreds of near-identical ones a suite produces. Stamps both onto the trace: `thread_source`, and each `sourceContext` entry as `source_context.<key>`. Two rules make an arbitrary caller bag safe to merge: - PREFIXED, because `buildBaseMetadata` spreads caller metadata LAST — an unprefixed `user_id` in a sourceContext would replace the real one. - FLAT AND SCALAR, because LangSmith filters match a metadata KEY, not a path into a JSON document; a nested object is unfilterable, so dropping it beats shipping something that cannot be queried. Entry count is bounded too. Stamped on all three trace paths — the message turn, the orchestrator resume, and the approval resume. That matters: a build finishes on a RESUME beat, so stamping only the message turn would leave the spans that contain the actual work unattributable. The harness now sends the case slug and iteration, so a traced eval run can be grouped by case: `fileSlug`/`iteration` ride BuildArgs to `ensureThread` as sourceContext. BuildArgs is pinned by a test for the reason the credentialFixture comment gives — `wrap()` erases the callback's parameter type, so a dropped field still type-checks. Reading the provenance is best-effort: a failed metadata read costs the trace a label, never the run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
n8n-assistant
Bot
added
cla-signed
core
Contributor
PR review overviewBased on ownership of the 10 changed files in this PR:
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 10 files
Architecture diagram
sequenceDiagram
participant Harness as Eval Harness (Orchestrator)
participant Client as N8n API Client
participant AI as InstanceAiService
participant Mem as MemoryService / DB
participant Trace as LangSmith / Tracing
Note over Harness,Trace: Phase 1: Thread Provisioning (Eval Harness)
Harness->>Client: NEW: ensureThread(threadId, projectId, sourceContext)
Note right of Harness: sourceContext contains evalCase & evalIteration
Client->>Mem: POST /threads (persists source + sourceContext)
Note over Harness,Trace: Phase 2: Run Execution or Resume (Server-Side)
alt Message Turn
AI->>AI: executeRun()
else Orchestrator/Approval Resume
AI->>AI: resumeSuspendedRun()
end
AI->>Mem: NEW: getThreadMetadata(userId, threadId)
alt Read Successful
Mem-->>AI: { source, sourceContext: { ... } }
AI->>AI: NEW: threadProvenanceMetadata()
Note right of AI: Maps to 'thread_source' and 'source_context.*'<br/>Filters: Scalars only, Max 20 keys
else Read Fails (Best Effort)
Mem-->>AI: Error (DB down/missing)
AI->>AI: NEW: Catch error & return empty metadata
end
AI->>Trace: NEW: createTraceContext(..., metadata: provenance)
alt Success Path
Trace-->>AI: traceId
Note over AI,Trace: Trace now filterable by entry point or eval case
else Unhappy Path (Tracing)
Trace-->>AI: Error
Note over AI: AI run continues (provenance is non-blocking)
end
AI-->>Harness: Return Run Result
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Contributor
Instance AI Workflow EvalImportant This eval does not re-run on new commits. To test your latest push, re-run it against the PR head: gh workflow run ci-instance-ai-evals.yml -f pr=36909…or use the Run workflow button and set pr = Tip 🟢 All 25 units green over 3 runs (clean). Gate: pass@k = 100% (every unit passes at least once across k runs) — 100.0% pass (75/75 trials over 25 units · k=3) pass@3 100.0% · pass^3 100.0% · LangSmith experiment Per-test-case results (8)
Workflow checksScored over 24 successful build(s). N/A = check did not apply to that workflow. Error = check could not be measured (e.g. judge timeout).
All workflow checks (2 failing of 35 checks)
|
Contributor
Instance AI Discovery Eval ✅Branch: Eval output |
…ts keep
65 lines for a two-field projection, and half of it defended against inputs the
API boundary already bounds:
- The 20-key cap silently dropped valid entries under the documented 2 KB
limit, which is worse than not capping (review finding). `sourceContext` is
size-capped where it enters; nothing more is needed here.
- The scalar-only filter was justified by LangSmith filterability, not
correctness, and every real caller already sends ids —
`{templateId, templateName}`, `{agentId}`, `{workflowId, executionId}`.
`sanitizeTraceValue` handles anything else downstream.
- THREAD_SOURCE_KEY / SOURCE_CONTEXT_PREFIX were exported and used nowhere.
What remains is the one rule that is load-bearing: the `source_context.`
prefix, because `buildBaseMetadata` spreads caller metadata LAST and a bare
`user_id` in a sourceContext would replace the real one. Its test stays.
32 lines, 4 tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/@n8n/instance-ai/src/tracing/thread-provenance.ts">
<violation number="1" location="packages/@n8n/instance-ai/src/tracing/thread-provenance.ts:28">
P2: When a caller supplies an object or array in `sourceContext`, this forwards it as one trace value. `toTelemetryAttributeValue` converts non-scalar values to JSON strings, so `source_context.<key>` is not filterable as the provenance feature promises; keep only scalar values or flatten them before emission.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| if (typeof source === 'string' && source) out.thread_source = source; | ||
| if (typeof sourceContext === 'object' && sourceContext !== null) { | ||
| for (const [key, value] of Object.entries(sourceContext)) { | ||
| out[`source_context.${key}`] = value; |
Contributor
There was a problem hiding this comment.
P2: When a caller supplies an object or array in sourceContext, this forwards it as one trace value. toTelemetryAttributeValue converts non-scalar values to JSON strings, so source_context.<key> is not filterable as the provenance feature promises; keep only scalar values or flatten them before emission.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/@n8n/instance-ai/src/tracing/thread-provenance.ts, line 28:
<comment>When a caller supplies an object or array in `sourceContext`, this forwards it as one trace value. `toTelemetryAttributeValue` converts non-scalar values to JSON strings, so `source_context.<key>` is not filterable as the provenance feature promises; keep only scalar values or flatten them before emission.</comment>
<file context>
@@ -1,65 +1,32 @@
- if (!isScalar(value)) continue;
- out[`${SOURCE_CONTEXT_PREFIX}${key}`] = value;
- taken++;
+ out[`source_context.${key}`] = value;
}
}
</file context>
Suggested change
| out[`source_context.${key}`] = value; | |
| \t\t\tif (typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean') {\n\t\t\t\tout[`source_context.${key}`] = value;\n\t\t\t} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A thread already records where it was opened from:
source(required, fromINSTANCE_AI_THREAD_SOURCES) plus the opener's ownsourceContextbag, both persisted in the thread's metadata byensureThread. Neither ever reached the trace.This stamps them on:
thread_source, and eachsourceContextentry assource_context.<key>.So a LangSmith project can now answer "which entry point produced this run?" — canvas vs template vs node-error vs evals — which it could not before. Today's callers make that concrete:
{templateId, templateName},{agentId},{workflowId, executionId}, and from the eval harness{evalCase, evalIteration}.Why now
The LangTracer eval harness is enabling builder tracing on its eval containers. Every build in that project is the same handful of test cases run over and over, and a trace carries only a bare-UUID
thread_id— so the conversations are indistinguishable and no export can group them.sourceContextis the existing hook for exactly this.Design
One rule is load-bearing: the
source_context.prefix.buildBaseMetadataspreads caller metadata last, so an unprefixeduser_idin a sourceContext would replace the real one. There is a test for that specifically.Nothing else is bounded here —
sourceContextis size-capped where it enters the API, andsanitizeTraceValuehandles values downstream. (An earlier revision also capped key count and filtered to scalars; both were dropped as redundant, and the cap silently dropped valid entries under the documented limit.)Stamped on all three trace paths — message turn, orchestrator resume, approval resume. This is load-bearing: a build finishes on a resume beat, so stamping only the message turn would leave the spans containing the actual work unattributable.
Reading the provenance is best-effort — a failed metadata read costs the trace a label, never the run.
Harness side
fileSlug/iterationrideBuildArgs→buildWorkflow→ensureThread.BuildArgsgets a pin test for the same reason thecredentialFixturecomment gives:wrap()erases the callback's parameter type, so a dropped field still type-checks.Verification
tracedBuildcall, and removing the stamp from the resume path, each turn their test red; restoring turns it green.packages/@n8n/instance-ai: tsc clean, full evaluations + tracing suites pass.packages/cli: tsc clean, instance-ai service suite 166 pass.@n8n/instance-aiin the cli service tests needed the new export added — without it the function isundefinedand...undefinedspreads to nothing, which reads as a passing test with silently empty metadata.🤖 Generated with Claude Code