Skip to content

Preserve workflow context across child fibers - #1327

Closed
yorzi wants to merge 1 commit into
crmne:mainfrom
yorzi:fix/workflow-child-fiber-context
Closed

yorzi wants to merge 1 commit into
crmne:mainfrom
yorzi:fix/workflow-child-fiber-context

Conversation

@yorzi

@yorzi yorzi commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

What this does

Events from a child fiber, Async task, or thread started inside RubyLLM.workflow lost their workflow and step, because Support::Instrumentation kept the context in fiber-local Thread.current[...]. Their usage attempts reached the ledger without the workflow they ran in.

The workflow context now lives in fiber storage, the way ProgressReporter keeps the progress listener and Accounting::Usage keeps the owner, so children inherit the workflow and step active when they are created. Workflow contexts are frozen hashes, so a child that enters its own step replaces its copy without touching its parent or siblings. The MCP listener thread clears the inherited workflow, as #1186 does for the usage owner, so notifications are not attributed to the workflow that happened to start the listener. The concurrent tool executor keeps its explicit propagation unchanged.

The instrumentation guide now says that children inherit the context active when they are created, and that background jobs still re-enter RubyLLM.workflow with the persisted ID.

The new examples cover:

  • Red before the fix: a child fiber gets the workflow and step; a workflow nested inside a child fiber links to its parent step and restores it after an error; a child thread gets the workflow without leaking it into later work; an Async embedding attempt is attributed to its workflow and step exactly once.
  • Guards: overlapping Async steps stay isolated from their siblings and the caller, Fiber.new(storage: nil) opts out, and the listener clears the workflow on every start.

How to reproduce

Run from the repository root with bundle exec ruby -Ilib repro.rb. Nothing goes over the network.

require 'ruby_llm'

events = []
instrumenter = Object.new
instrumenter.define_singleton_method(:instrument) do |name, payload, &block|
  events << [name, payload[:workflow_id], payload[:workflow_step_id]]
  block&.call(payload)
end
context = RubyLLM.context do |config|
  config.openai_api_key = 'unused-offline'
  config.instrumenter = instrumenter
end
chat = context.chat(model: 'gpt-4.1')
reply = RubyLLM::Message.new(role: :assistant, content: 'Done', input_tokens: 1, output_tokens: 1)
chat.provider.define_singleton_method(:complete) { |*, **| reply }

context.workflow('Search', id: 'search-1') do |workflow|
  workflow.step('Summarize', id: 'summarize-1') do
    Fiber.new { chat.ask('Summarize') }.resume
  end
end
events.uniq.each { |name, workflow, step| puts "#{name}: workflow=#{workflow.inspect} step=#{step.inspect}" }

Before:

workflow.ruby_llm: workflow="search-1" step=nil
workflow_step.ruby_llm: workflow="search-1" step="summarize-1"
chat.ruby_llm: workflow=nil step=nil
usage.ruby_llm: workflow=nil step=nil

After:

workflow.ruby_llm: workflow="search-1" step=nil
workflow_step.ruby_llm: workflow="search-1" step="summarize-1"
chat.ruby_llm: workflow="search-1" step="summarize-1"
usage.ruby_llm: workflow="search-1" step="summarize-1"

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Performance improvement

Scope check

  • I read the Contributing Guide
  • This aligns with RubyLLM's focus on LLM communication
  • This isn't application-specific logic that belongs in user code
  • This benefits most users, not just my specific use case

Related issue

Fixes #1323

Quality check

  • I ran overcommit --install and all hooks pass
  • I tested my changes thoroughly
    • For provider changes: Re-recorded VCR cassettes with bundle exec rake vcr:record[provider_name]
    • All tests pass: bundle exec rspec --tag '~live' --tag '~generator' (6030 examples; the only 2 failures, in spec/ruby_llm/sandbox/mxc_spec.rb, fail the same way on main on macOS)
  • I updated documentation if needed (docs/_advanced/instrumentation.md describes inheritance by child fibers and threads)
  • I didn't modify auto-generated files manually (models.json, aliases.json)

Ownership

  • I understand every line of this change and can answer questions about it

API changes

  • Breaking change
  • New public methods/classes
  • Changed method signatures
  • No API changes

Summary by CodeRabbit

  • New Features
    • Workflow and step context now carries into child fibers and threads, including concurrent tool execution. Child workflows remain isolated from their parent and siblings, while retaining the appropriate parent relationship.
    • Workflow context is restored after a block finishes or raises an error, and does not leak into later work.
  • Documentation
    • Clarified that background jobs and separate processes do not inherit workflow context automatically; they must re-enter a workflow using its persisted ID.

Application-created fibers lose workflow and step attribution even
when their calls belong to the enclosing workflow. Use inheritable
fiber storage and clear workflow context in independent MCP listeners.

Cover nested and concurrent work, usage attribution, and listener
restart without changing the public workflow API.

Fixes crmne#1323
@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 59df5a0 and 783357c.

Files selected for processing: 6
  • docs/_advanced/instrumentation.md
  • lib/ruby_llm/mcp/listener.rb
  • lib/ruby_llm/support/instrumentation.rb
  • spec/ruby_llm/accounting/usage_workflow_spec.rb
  • spec/ruby_llm/mcp/listener_spec.rb
  • spec/ruby_llm/workflow_spec.rb

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Walkthrough

Workflow context now uses fiber-local storage. Child fibers and threads inherit workflow context, while the MCP listener thread starts with no workflow context. Specs and documentation cover propagation, isolation, and context restoration.

Changes: Workflow Context Propagation

Layer / File(s) Summary
Fiber-local context and propagation
lib/ruby_llm/support/instrumentation.rb, spec/ruby_llm/workflow_spec.rb, spec/ruby_llm/accounting/usage_workflow_spec.rb, docs/_advanced/instrumentation.md
Workflow context now uses Fiber[] and restores the caller’s prior context. Specs cover child fibers, threads, concurrent steps, usage attribution, and disabled inheritance. Documentation describes child context and the limits of propagation.
Listener context reset
lib/ruby_llm/mcp/listener.rb, spec/ruby_llm/mcp/listener_spec.rb
The listener thread runs with a nil workflow context. A spec checks that callbacks see no current workflow across two starts.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

  • crmne/ruby_llm#1300: Usage entries persist workflow fields from the instrumentation context that this PR changes.

Suggested labels: bug, documentation

Suggested reviewers: crmne, kieranklaassen

Merge Risk: Minimal · up to 78335

No concrete merge-blocking issue is established by the supplied evidence. Merge after normal checks.

Pre-merge checks: Passed 6 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 6 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: preserving workflow context across child fibers.
Description check Passed The description is complete and relevant. It explains the change, scope, testing, documentation, related issue, known baseline failures, ownership, and API impact.
Linked Issues check Passed Requirements from #1323 (direct link, open):

1. Child fibers, Async tasks, and threads keep the enclosing workflow and step. Met. lib/ruby_llm/support/instrumentation.rb now stores the context with…
Out of Scope Changes check Passed Changes against the merge base 59df5a08, reviewed at head 783357c4:

- lib/ruby_llm/support/instrumentation.rb: storage switched from Thread.current to Fiber[]. This implements #1323 directl…
Review Evidence Stays Out Of The Repository Passed The pull request changes only one Markdown documentation file and five Ruby files. All six files are existing files; no screenshots, captures, recordings, comparison pages, media assets, or HTML evide…
Dependencies From Sources The Maintainer Controls Passed PASS. The PR changes only documentation, library code, and specs. It does not modify a manifest, lockfile, or package-manager configuration. The added require 'async/queue' uses the existing async…
Full details: Linked Issues check

Explanation

Requirements from #1323 (direct link, open):

  1. Child fibers, Async tasks, and threads keep the enclosing workflow and step. Met. lib/ruby_llm/support/instrumentation.rb now stores the context with Fiber[WORKFLOW_CONTEXT_KEY] in current_workflow and with_workflow (previously Thread.current[...]). Fiber storage is copied into child fibers and threads at creation, so children inherit the active workflow and step. instrument merges that context into payloads, so chat.ruby_llm and usage.ruby_llm events from children carry workflow_id and workflow_step_id.
  2. Sibling steps stay isolated and the caller's context is restored. Met. with_workflow saves the previous value and restores it in ensure. Each child that enters its own step replaces its own copy, and the context hashes are frozen, so parent and sibling values are not changed. The PR adds specs for overlapping Async steps and for parent-step restoration after errors (spec/ruby_llm/workflow_spec.rb, per the summary). I did not read these spec bodies. The result rests on the implementation and the PR report.
  3. The MCP listener clears inherited context, as #1186 does for the usage owner. Met. lib/ruby_llm/mcp/listener.rb wraps the listener thread body in Support::Instrumentation.with_workflow(nil). The listener spec (per the summary) checks that callbacks see no workflow on each start.
  4. Background jobs re-enter with the persisted ID. This is out of automatic inheritance scope and needs no code change. The instrumentation guide was updated to say so (docs/_advanced/instrumentation.md, per the summary).

No non-coding requirements apply to this issue. The PR reports a full test run with two failures in spec/ruby_llm/sandbox/mxc_spec.rb that the PR says also fail on main. I did not run the suite.

Full details: Out of Scope Changes check

Explanation

Changes against the merge base 59df5a08, reviewed at head 783357c4:

  • lib/ruby_llm/support/instrumentation.rb: storage switched from Thread.current to Fiber[]. This implements #1323 directly.
  • lib/ruby_llm/mcp/listener.rb: listener thread runs inside with_workflow(nil), which implements the MCP-clearing requirement of #1323. Existing ProgressReporter.listen and Accounting::Usage.with_owner wrappers are kept.
  • docs/_advanced/instrumentation.md: documents child inheritance and the background-job re-entry rule. This supports the behavior change.
  • Specs in spec/ruby_llm/workflow_spec.rb, spec/ruby_llm/accounting/usage_workflow_spec.rb, and spec/ruby_llm/mcp/listener_spec.rb: regression coverage for the changed behavior.

The PR states the concurrent tool executor keeps its explicit propagation unchanged. No diff hunk touches it, which matches that statement. No change is unrelated to #1323. Public API signatures are unchanged.

Full details: Review Evidence Stays Out Of The Repository

Explanation

The pull request changes only one Markdown documentation file and five Ruby files. All six files are existing files; no screenshots, captures, recordings, comparison pages, media assets, or HTML evidence files are added. The documentation and specs contain ordinary workflow examples and tests, not reviewer evidence.

Full details: Dependencies From Sources The Maintainer Controls

Explanation

PASS. The PR changes only documentation, library code, and specs. It does not modify a manifest, lockfile, or package-manager configuration. The added require 'async/queue' uses the existing async development dependency, which is unchanged in both base and head. No new git URL, fork, patch, override, replace entry, or alternate registry is introduced.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 5 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the bug Something isn't working label Oct 27, 2026
@github-actions
github-actions Bot requested a review from crmne October 27, 2026 06:58
@crmne crmne closed this in c19d898 Oct 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Work inside a child fiber loses its workflow and step

2 participants