Skip to content

[codex] normalize pipeline completed text value - #1415

Merged
qdaxb merged 1 commit into
mainfrom
codex/pipeline-normalize-completed-value
Jun 16, 2026
Merged

qdaxb merged 1 commit into
mainfrom
codex/pipeline-normalize-completed-value

Conversation

@qdaxb

@qdaxb qdaxb commented Jun 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Normalize completed execution results so an empty result.value is populated from finalized text blocks when accumulated streaming text is unavailable.
  • Keep pipeline context passing on the normal result.value path instead of teaching context consumers to inspect block internals.
  • Add lifecycle regression coverage for online-style completed results where visible assistant output exists only in text blocks.

Root Cause

Online callback handling can finalize visible assistant output into result.blocks while result.value remains empty, especially across callback/streaming state boundaries. Pipeline auto-advance then correctly creates the next user handoff message, but the handoff text is blank because the completed result was not normalized at the lifecycle boundary.

Block Handling

  • text blocks may be used to populate result.value only when the standard text value is blank.
  • thinking blocks remain in result.blocks for UI display and are not sent as pipeline handoff text.
  • tool blocks remain in result.blocks for UI/tool state and are not sent as pipeline handoff text.

Validation

  • uv run pytest tests/services/chat/trigger/test_lifecycle_completed_result.py tests/services/chat/storage/test_pipeline_context_passing.py tests/services/adapters/test_collaboration_strategy.py tests/services/chat/storage/test_pipeline_auto_advance_intent.py tests/services/chat/storage/test_task_manager.py tests/services/execution/test_status_updating_emitter.py -q
  • uv run black --check app/services/chat/trigger/lifecycle.py tests/services/chat/trigger/test_lifecycle_completed_result.py
  • uv run isort --check-only --diff app/services/chat/trigger/lifecycle.py tests/services/chat/trigger/test_lifecycle_completed_result.py
  • git diff --check
  • uv run pytest -q (2900 passed, 1 skipped)

Summary by CodeRabbit

  • Bug Fixes
    • Improved result value population in chat completions to more robustly handle empty values, now normalizing them to extracted text content from available blocks when accumulated content is unavailable.

@coderabbitai

coderabbitai Bot commented Jun 16, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c5ae94e4-0c71-4aca-8e2c-0853000108e3

📥 Commits

Reviewing files that changed from the base of the PR and between 736ed28 and 61026af.

📒 Files selected for processing (2)
  • backend/app/services/chat/trigger/lifecycle.py
  • backend/tests/services/chat/trigger/test_lifecycle_completed_result.py

📝 Walkthrough

Walkthrough

Two internal helpers (_is_blank_text_value, _extract_text_value_from_blocks) are added to lifecycle.py. collect_completed_result is updated to use them: it now fills final_result["value"] when blank by preferring accumulated_content and falling back to text extracted from final_result["blocks"]. A new test and helper class cover the block-extraction fallback.

Changes

Blank-aware result value normalization

Layer / File(s) Summary
Blank detection helpers and updated collect_completed_result logic
backend/app/services/chat/trigger/lifecycle.py
Adds _is_blank_text_value (treats None/whitespace as blank) and _extract_text_value_from_blocks (joins non-empty content/text fields from type=="text" blocks). Updates collect_completed_result to set final_result["value"] when blank: prefers non-blank accumulated_content, falls back to extracted block text.
Test: empty value normalized from text blocks
backend/tests/services/chat/trigger/test_lifecycle_completed_result.py
Adds _TextBlockSessionManager (empty accumulated content, one finalized text block) and an async test asserting collect_completed_result normalizes result={"value": ""} to the block's content string.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A blank little value came hoppin' along,
With no content inside — something clearly was wrong.
So I sniffed through the blocks with my floppy-eared care,
Found the text hiding there, tucked away with a flair.
Now no empty result shall escape my review!
🥕 Every value gets filled — hippity-hoo!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/pipeline-normalize-completed-value

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 and usage tips.

@qdaxb
qdaxb marked this pull request as ready for review June 16, 2026 07:13
@qdaxb
qdaxb merged commit d7aa082 into main Jun 16, 2026
20 of 21 checks passed
@qdaxb
qdaxb deleted the codex/pipeline-normalize-completed-value branch June 16, 2026 07:14
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