Repository navigation
fix(inngest): keep experiment scores run-scoped when runId is undefined - #1880
princejain756 wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 842034e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
[Medium impact] The changes affect score targeting in two SDK score-writing workflows. The PR appears safe to merge, with a non-blocking update needed to the public experiment score docs. Findings
Fix with agent prompt### Issue 1
packages/inngest/src/components/InngestScore.ts:211-213
The public docs for `ClientScore.experiment` at lines 50–53 still warn that calling inside `step.run()` without `runId` produces a step-scoped write. This fix makes that call run-scoped, so the editor docs now tell developers to avoid a supported call. Remove the warning and explain that an omitted `runId` uses the current run, including inside `step.run()`.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryKeeps experiment scores attached to a run when
One non-blocking docs update remains: the public API comment still describes the old behavior. Reviews (1) · Last reviewed commit: "fix(inngest): keep experiment scores run..." · Reviewed by Greptile |
| // Experiment scores are always run-scoped; null keeps current-run lookup | ||
| // intact even when called inside step.run(). | ||
| const target = { runId: options.runId ?? null }; |
There was a problem hiding this comment.
Docs describe the old behavior
The public docs for ClientScore.experiment at lines 50–53 still warn that calling inside step.run() without runId produces a step-scoped write. This fix makes that call run-scoped, so the editor docs now tell developers to avoid a supported call. Remove the warning and explain that an omitted runId uses the current run, including inside step.run().
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/inngest/src/components/InngestScore.ts
Line: 211-213
Comment:
**Docs describe the old behavior**
The public docs for `ClientScore.experiment` at lines 50–53 still warn that calling inside `step.run()` without `runId` produces a step-scoped write. This fix makes that call run-scoped, so the editor docs now tell developers to avoid a supported call. Remove the warning and explain that an omitted `runId` uses the current run, including inside `step.run()`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Fixes #1848 (EXE-2309)
score.experiment()rejectsstepIdbecause experiment scores are always run-scoped, butsendScoreExperimentbuilttargetas{ runId: options.runId }instead of{ runId: options.runId ?? null }(unlikesendStepScore), causingInngestMetadata.performOpto fall back tostepscope when called insidestep.run()withoutrunId. IncreateScorer, spreading...runResult/...resultafterrunId: parent.runIdcould also overwriteparent.runIdwhen the scorer handler returnedrunId: undefined.This uses
options.runId ?? nullinsendScoreExperimentand resolvesrunIdwith?? parent.runIdincreateScorer.