Refactor generation services for single configured extractor - #101
Merged
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR converts extractor/evaluator config fields from plural lists to singular nullable objects across frontend and backend, refactors generation orchestration to run a single configured extractor per cycle, updates API/configurator aliasing and PATCH semantics, and adjusts UI and tests to the new shape. ChangesSingular Extractor Config Refactor
Estimated code review effort: 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
reflexio/server/services/base_generation_service.py (1)
1108-1126: 💤 Low valueMinor inconsistency in deprecated list-handling path.
When
_collect_scoped_interactions_for_precheckis called with a list directly (bypassing_should_run_before_extraction), the returnedscoped_configwill be the last config in the list rather than the first, due to the loop reassigning it on each iteration. This is inconsistent with the "first-entry-wins" semantics documented elsewhere.Since this path is deprecated and the main flow normalizes to a single config before calling this method (at lines 960-963), this has limited impact. However, if any legacy callers rely on the list interface directly, they might see unexpected behavior.
Optional fix for consistency
if isinstance(extractor_config, list): deduped_sessions: dict[str, RequestInteractionDataModel] = {} scoped_config = extractor_config[0] if extractor_config else None + first_config = scoped_config for config in extractor_config: session_data_models, scoped_config = ( self._collect_scoped_interactions_for_precheck(config) ) for data_model in session_data_models: request_id = getattr(data_model.request, "request_id", None) dedupe_key = ( request_id or data_model.session_id or f"scoped_group_{len(deduped_sessions)}" ) if dedupe_key not in deduped_sessions: deduped_sessions[dedupe_key] = data_model - if scoped_config is None: + if first_config is None: raise ValueError("extractor_config list must not be empty") - return list(deduped_sessions.values()), scoped_config + return list(deduped_sessions.values()), first_config🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@reflexio/server/services/base_generation_service.py` around lines 1108 - 1126, The current loop in the list-handling branch of _collect_scoped_interactions_for_precheck reassigns scoped_config on every iteration so the final scoped_config becomes the last config in extractor_config; change the logic so scoped_config preserves the first non-None value (first-entry-wins). Specifically, when iterating extractor_config, call self._collect_scoped_interactions_for_precheck(config) to collect session_data_models but only assign scoped_config from that call if scoped_config is currently None (or initialize scoped_config=None and set it on the first iteration), leaving deduped_sessions aggregation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/hooks/use-settings.tsx`:
- Around line 38-44: The lazy initializer useState(() => loadSettings()) runs on
server and client causing SSR hydration mismatch; change settings initialization
to a server-safe constant (e.g., useState<Settings>({ apiEndpoint:
"http://localhost:8081" })) and move localStorage read into a client-only
useEffect that runs after mount to call setSettings(loadSettings()). Add a
useRef flag (e.g., isInitialMount) to prevent the persistence effect that writes
to localStorage (using STORAGE_KEY) from immediately overwriting stored values
on first render, and keep the existing effect that writes settings to
localStorage but guard it so it only runs after the client load has completed.
In `@reflexio/server/api.py`:
- Around line 1731-1737: The fallback to legacy_configs[0] isn't validated
before reading success_config.evaluation_name, which can raise AttributeError;
after assigning success_config from legacy_configs ensure the object has a valid
string evaluation_name (e.g., check isinstance(getattr(success_config,
"evaluation_name", None), str)) before using it, and build known safely (e.g.,
use getattr(success_config, "evaluation_name", None) and only add it if it's a
str) so that the variables success_config, legacy_configs, evaluation_name and
known are handled defensively.
---
Nitpick comments:
In `@reflexio/server/services/base_generation_service.py`:
- Around line 1108-1126: The current loop in the list-handling branch of
_collect_scoped_interactions_for_precheck reassigns scoped_config on every
iteration so the final scoped_config becomes the last config in
extractor_config; change the logic so scoped_config preserves the first non-None
value (first-entry-wins). Specifically, when iterating extractor_config, call
self._collect_scoped_interactions_for_precheck(config) to collect
session_data_models but only assign scoped_config from that call if
scoped_config is currently None (or initialize scoped_config=None and set it on
the first iteration), leaving deduped_sessions aggregation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a787ba57-e101-4bd3-a5fb-8df597e248f1
📒 Files selected for processing (16)
docs/components/configure/config-editor.tsxdocs/components/configure/sections.tsxdocs/components/method/code-panel.tsxdocs/hooks/use-settings.tsxdocs/lib/config-schema.tsreflexio/models/api_schema/eval_overview_schema.pyreflexio/models/config_schema.pyreflexio/server/api.pyreflexio/server/services/agent_success_evaluation/agent_success_evaluation_service.pyreflexio/server/services/base_generation_service.pyreflexio/server/services/configurator/base_configurator.pyreflexio/server/services/playbook/playbook_generation_service.pyreflexio/server/services/profile/profile_generation_service.pytests/models/test_validators.pytests/server/services/agent_success_evaluation/test_agent_success_evaluation_services.pytests/server/services/test_base_generation_service.py
Comment on lines
+1731
to
+1737
| success_config = getattr(config, "agent_success_config", None) | ||
| if success_config is None or not isinstance( | ||
| getattr(success_config, "evaluation_name", None), str | ||
| ): | ||
| legacy_configs = getattr(config, "agent_success_configs", None) | ||
| success_config = legacy_configs[0] if legacy_configs else None | ||
| known = {success_config.evaluation_name} if success_config else set() |
There was a problem hiding this comment.
Validate legacy config before accessing evaluation_name.
The canonical agent_success_config is validated for a string evaluation_name (lines 1732-1734), but when falling back to legacy_configs[0] (line 1736), no validation is applied. If the legacy config exists but lacks a valid evaluation_name, line 1737 will raise AttributeError when accessing success_config.evaluation_name.
🛡️ Recommended defensive fix
Add validation after the legacy fallback or use getattr defensively:
success_config = getattr(config, "agent_success_config", None)
if success_config is None or not isinstance(
getattr(success_config, "evaluation_name", None), str
):
legacy_configs = getattr(config, "agent_success_configs", None)
success_config = legacy_configs[0] if legacy_configs else None
+ # Re-validate legacy config
+ if success_config and not isinstance(
+ getattr(success_config, "evaluation_name", None), str
+ ):
+ success_config = None
known = {success_config.evaluation_name} if success_config else set()Alternatively, use defensive attribute access:
-known = {success_config.evaluation_name} if success_config else set()
+known = (
+ {success_config.evaluation_name}
+ if success_config and isinstance(
+ getattr(success_config, "evaluation_name", None), str
+ )
+ else set()
+)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| success_config = getattr(config, "agent_success_config", None) | |
| if success_config is None or not isinstance( | |
| getattr(success_config, "evaluation_name", None), str | |
| ): | |
| legacy_configs = getattr(config, "agent_success_configs", None) | |
| success_config = legacy_configs[0] if legacy_configs else None | |
| known = {success_config.evaluation_name} if success_config else set() | |
| success_config = getattr(config, "agent_success_config", None) | |
| if success_config is None or not isinstance( | |
| getattr(success_config, "evaluation_name", None), str | |
| ): | |
| legacy_configs = getattr(config, "agent_success_configs", None) | |
| success_config = legacy_configs[0] if legacy_configs else None | |
| # Re-validate legacy config | |
| if success_config and not isinstance( | |
| getattr(success_config, "evaluation_name", None), str | |
| ): | |
| success_config = None | |
| known = {success_config.evaluation_name} if success_config else set() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@reflexio/server/api.py` around lines 1731 - 1737, The fallback to
legacy_configs[0] isn't validated before reading success_config.evaluation_name,
which can raise AttributeError; after assigning success_config from
legacy_configs ensure the object has a valid string evaluation_name (e.g., check
isinstance(getattr(success_config, "evaluation_name", None), str)) before using
it, and build known safely (e.g., use getattr(success_config, "evaluation_name",
None) and only add it if it's a str) so that the variables success_config,
legacy_configs, evaluation_name and known are handled defensively.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
reflexio/server/api.py (1)
1220-1228:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject/normalize deprecated legacy PATCH keys in
/api/update_configbeforeConfig(**merged)
update_config()shallow-mergespartialover the existing config, butreflexio.models.config_schema.Configdoes not forbid/validate unknown extra top-level fields—so legacy keys likeprofile_extractor_configs,user_playbook_extractor_configs, andagent_success_configsare ignored duringConfig(**merged). The endpoint can therefore return success while silently dropping the intended update (no migration/422 path).Suggested fix
reflexio = get_reflexio(org_id=org_id) existing = reflexio.request_context.configurator.get_config().model_dump( mode="python" ) - merged = {**existing, **partial} + legacy_keys = { + "profile_extractor_configs": "profile_extractor_config", + "user_playbook_extractor_configs": "user_playbook_extractor_config", + "agent_success_configs": "agent_success_config", + } + normalized_partial = dict(partial) + for old, new in legacy_keys.items(): + if old in normalized_partial and new not in normalized_partial: + legacy_value = normalized_partial.pop(old) + normalized_partial[new] = ( + legacy_value[0] if isinstance(legacy_value, list) else legacy_value + ) + + merged = {**existing, **normalized_partial}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@reflexio/server/api.py` around lines 1220 - 1228, In update_config(), before calling Config(**merged) on the merged dict, detect any deprecated top-level keys (e.g. "profile_extractor_configs", "user_playbook_extractor_configs", "agent_success_configs") present in merged and either normalize them to the new shape or reject them with a 422 client error; implement by checking merged.keys(), collecting any deprecated keys found, and if any exist return a 422 response (with a clear message listing the deprecated keys) instead of proceeding to Config(**merged) so updates don’t silently drop legacy fields.tests/server/services/test_base_generation_service.py (1)
1136-1137:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winForward
extractor_configsby keyword to avoid binding it toextractor_config.The parent signature is now
__init__(self, llm_client, request_context, extractor_config=None, extractor_configs=None). Passingextractor_configspositionally binds the list to the parent'sextractor_configparameter instead. Since a non-empty list is notNone, the parent'sif extractor_config is not None:branch assignsself._extractor_config = [MockExtractorConfig(...)](the whole list) rather than the first entry.This is currently masked because
InProgressTrackingService._run_generationis overridden to only bump a counter and never consumes_extractor_config, but it will silently produce a list-valued_extractor_configfor any subclass test that relies on the loaded config.🐛 Proposed fix
def __init__(self, llm_client, request_context, extractor_configs=None): - super().__init__(llm_client, request_context, extractor_configs) + super().__init__(llm_client, request_context, extractor_configs=extractor_configs) self._generation_count = 0 # Tracks _run_generation calls🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/server/services/test_base_generation_service.py` around lines 1136 - 1137, The subclass __init__ currently calls super().__init__(llm_client, request_context, extractor_configs) which passes extractor_configs positionally and accidentally binds it to the parent's extractor_config parameter; change the call to forward extractor_configs by keyword (super().__init__(llm_client, request_context, extractor_configs=extractor_configs)) so the parent receives extractor_config and extractor_configs correctly and the parent's logic that checks if extractor_config is not None behaves as intended.
🧹 Nitpick comments (2)
docs/components/configure/config-editor.tsx (1)
34-43: ⚡ Quick winConsider explicitly normalizing all extractor config fields.
The
hydratefunction explicitly normalizesagent_success_configto ensure it's either the incoming value ornull, but relies on the spread operator forprofile_extractor_configanduser_playbook_extractor_config. If the server omits these fields (returningundefinedin the deserialized JSON), they would remainundefinedin the merged config rather than becomingnull.For consistency and type safety, consider explicitly normalizing all singular extractor config fields:
function hydrate(raw: unknown): ReflexioConfig { const base = defaultConfig(); if (!raw || typeof raw !== "object") return base; const incoming = raw as Partial<ReflexioConfig>; return { ...base, ...incoming, agent_success_config: incoming.agent_success_config ?? null, + profile_extractor_config: incoming.profile_extractor_config ?? null, + user_playbook_extractor_config: incoming.user_playbook_extractor_config ?? null, }; }This ensures all extractor configs have consistent
null(notundefined) semantics when missing from the server response.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/components/configure/config-editor.tsx` around lines 34 - 43, The hydrate function currently spreads incoming onto base but only normalizes agent_success_config to null; update hydrate (which uses defaultConfig() and the incoming: Partial<ReflexioConfig> variable) to explicitly normalize all singular extractor fields—e.g., set profile_extractor_config: incoming.profile_extractor_config ?? null and user_playbook_extractor_config: incoming.user_playbook_extractor_config ?? null—so the returned ReflexioConfig never contains undefined for those extractor fields and retains the existing agent_success_config normalization.tests/server/services/profile/test_profile_generation_service.py (1)
214-271: ⚡ Quick winKeep coverage for the no-criteria fast path.
The single-config tests cover positive rendering paths, but the branch where
_build_should_run_prompt()should returnNoneis no longer exercised. A tiny case with an empty/whitespace-only definition and no override would lock in the guard that prevents useless should-run prompts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/server/services/profile/test_profile_generation_service.py` around lines 214 - 271, Add a test that asserts the fast-path returning None when there is no meaningful criteria: create a ProfileExtractorConfig with extractor_name (e.g., "none") and an extraction_definition_prompt set to "" or only whitespace and no should_extract_profile_prompt_override, call service._build_should_run_prompt(config, sample_request_interaction_models) (patch service.configurator.get_agent_context as in other tests) and assert the result is None to cover the no-criteria branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/lib/config-schema.ts`:
- Around line 138-140: The current defaultConfig seeds profile_extractor_config
and user_playbook_extractor_config using defaultProfileExtractor() /
defaultPlaybookExtractor(), causing enabled extractor objects with empty prompt
fields to be submitted; update the behavior so these fields are either set to
null by default (replace profile_extractor_config and
user_playbook_extractor_config defaults with null) or modify serializeConfig()
to normalize and collapse empty extractor objects into null by checking their
nested prompt/text fields (e.g., in serializeConfig(), detect if the extractor
prompt fields are empty/whitespace and replace the entire extractor object with
null before returning the payload) to prevent sending enabled-but-blank
extractor configs.
---
Outside diff comments:
In `@reflexio/server/api.py`:
- Around line 1220-1228: In update_config(), before calling Config(**merged) on
the merged dict, detect any deprecated top-level keys (e.g.
"profile_extractor_configs", "user_playbook_extractor_configs",
"agent_success_configs") present in merged and either normalize them to the new
shape or reject them with a 422 client error; implement by checking
merged.keys(), collecting any deprecated keys found, and if any exist return a
422 response (with a clear message listing the deprecated keys) instead of
proceeding to Config(**merged) so updates don’t silently drop legacy fields.
In `@tests/server/services/test_base_generation_service.py`:
- Around line 1136-1137: The subclass __init__ currently calls
super().__init__(llm_client, request_context, extractor_configs) which passes
extractor_configs positionally and accidentally binds it to the parent's
extractor_config parameter; change the call to forward extractor_configs by
keyword (super().__init__(llm_client, request_context,
extractor_configs=extractor_configs)) so the parent receives extractor_config
and extractor_configs correctly and the parent's logic that checks if
extractor_config is not None behaves as intended.
---
Nitpick comments:
In `@docs/components/configure/config-editor.tsx`:
- Around line 34-43: The hydrate function currently spreads incoming onto base
but only normalizes agent_success_config to null; update hydrate (which uses
defaultConfig() and the incoming: Partial<ReflexioConfig> variable) to
explicitly normalize all singular extractor fields—e.g., set
profile_extractor_config: incoming.profile_extractor_config ?? null and
user_playbook_extractor_config: incoming.user_playbook_extractor_config ??
null—so the returned ReflexioConfig never contains undefined for those extractor
fields and retains the existing agent_success_config normalization.
In `@tests/server/services/profile/test_profile_generation_service.py`:
- Around line 214-271: Add a test that asserts the fast-path returning None when
there is no meaningful criteria: create a ProfileExtractorConfig with
extractor_name (e.g., "none") and an extraction_definition_prompt set to "" or
only whitespace and no should_extract_profile_prompt_override, call
service._build_should_run_prompt(config, sample_request_interaction_models)
(patch service.configurator.get_agent_context as in other tests) and assert the
result is None to cover the no-criteria branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 868e051d-c620-49d2-a332-99d45dd70e06
📒 Files selected for processing (28)
docs/components/configure/config-editor.tsxdocs/components/configure/sections.tsxdocs/lib/config-schema.tsreflexio/models/api_schema/eval_overview_schema.pyreflexio/models/config_schema.pyreflexio/server/api.pyreflexio/server/services/agent_success_evaluation/agent_success_evaluation_service.pyreflexio/server/services/base_generation_service.pyreflexio/server/services/configurator/base_configurator.pyreflexio/server/services/configurator/test_config_storage.pyreflexio/server/services/playbook/playbook_aggregator.pyreflexio/server/services/playbook/playbook_generation_service.pyreflexio/server/services/profile/profile_generation_service.pytests/cli/test_bootstrap_config.pytests/lib/test_profile_workflows_unit.pytests/models/test_validators.pytests/server/api_endpoints/conftest.pytests/server/api_endpoints/test_api_routes.pytests/server/services/agent_success_evaluation/test_agent_success_evaluation_services.pytests/server/services/playbook/test_cluster_change_detection.pytests/server/services/playbook/test_playbook_aggregator.pytests/server/services/playbook/test_playbook_generation_service.pytests/server/services/playbook/test_playbook_generation_service_integration.pytests/server/services/profile/test_profile_generation_service.pytests/server/services/test_base_generation_service.pytests/server/services/test_configurator.pytests/server/services/test_profile_generation_service.pytests/server/services/test_profile_source_filtering.py
💤 Files with no reviewable changes (4)
- reflexio/server/services/playbook/playbook_aggregator.py
- reflexio/server/services/agent_success_evaluation/agent_success_evaluation_service.py
- reflexio/server/services/configurator/base_configurator.py
- tests/models/test_validators.py
✅ Files skipped from review due to trivial changes (1)
- reflexio/models/api_schema/eval_overview_schema.py
yyiilluu
force-pushed
the
yilu/single-extractor-generation-service
branch
from
May 29, 2026 04:18
fd0292b to
d121958
Compare
Add shared normalize_legacy_config_shape() to config_schema and apply it at the local-file storage load boundary so configs persisted before the single-extractor refactor recover their singular extractor/evaluator fields instead of silently dropping the legacy list keys. Drop the dead legacy *_extractor_configs fallback in resume_worker now that the computed list views are gone.
yilu331
added a commit
that referenced
this pull request
May 29, 2026
The previous merge commit (cad4947) shipped only the merge metadata; the backend-eng agent's actual conflict resolution work was stashed during diff comparison and never restaged before the commit landed. As a result, main's recent feature/refactor PRs (#98 cleanup, #99 Postgres pool config, #100 operation polling guard, #101 single configured extractor, #102 resumable extraction agent + pending tool call API) were silently absent from the merge commit. This commit applies the 113-file resolution that was on disk: - Config schema: agent_success_config singular form, normalize_legacy_ config_shape migration, pool_size/pool_acquire_timeout fields. - Service layer: _load_extractor_config singular accessors throughout the agent_success_evaluation services. - Storage: AgentRunMixin + SQLiteAgentRunMixin wired into the BaseStorage/ SQLiteStorage MRO; _migrate_agent_runs_schema + _migrate_pending_tool_ calls_schema run on SQLite startup. - Client: min_started_at param on _poll_operation_status; submitted_at threaded through the rerun_* paths. - API: pending_tool_call_api import + router registration; resume scheduler bootstrap. - Dashboard: _is_storage_configured guards on braintrust methods. - Braintrust: fail-closed Fernet, transport-error wrapping, client.close() in finally blocks. F1 additions (shadow_comparison_verdicts table, ShadowWinRateTrend, escalation_rate, per-turn judge, drawer endpoint) and F3 additions (sampler, concurrency, grade_on_demand) are preserved on top of main's structure. Sanity: - import reflexio OK. - tests/server/services/storage/: 239 passed.
This was referenced May 29, 2026
yilu331
added a commit
that referenced
this pull request
Jun 3, 2026
## What Fixes 3 stale tests in the profile-extraction suite that have been failing on `main`. **Test-only — no production code changes.** All three asserted contracts that two earlier refactors superseded; production is correct. ### Root cause - **#101** (per-extractor pre-check): `_collect_scoped_interactions_for_precheck` / `_should_run_before_extraction` moved from a *consolidated-across-all-extractors (list)* design to **per-extractor (single config)**. Two tests still passed a `list`, yielding `AttributeError: 'list' object has no attribute 'extraction_definition_prompt'`. - **#107** (scope async-info tools per extractor kind): profile extractors now expose `attach_pending_info_request`, not `ask_human`. The third test mocked an `ask_human` call for a profile extractor, so no tool ran → 0 pending calls. ### Changes 1. `test_collect_scoped_interactions_for_precheck_uses_extractor_scope` — single-config contract; still asserts extractor-specific window (`k=150`) + source (`["api"]`), and adds a non-matching-source → empty-groups (should_skip) assertion. 2. `test_should_run_before_extraction_combines_all_extractor_criteria` → renamed `..._includes_extractor_definition_and_override` — single-config; asserts the extractor's definition + override both reach the prompt. Dropped the obsolete cross-extractor assertion (criteria are no longer combined). 3. `test_ask_human_is_org_scoped_and_run_still_finalizes` → renamed `test_attach_pending_info_request_is_org_scoped_and_run_still_finalizes` — models the current profile attach-to-existing flow: seeds an org-scoped `ask_human` pending call, attaches via `attach_pending_info_request`, asserts org scope + run finalize + the run-tool-dependency edge (`pending_tool_call_ids` stays `[]` because attach returns a synchronous `Completed`, not `AsyncAccepted` — verified correct-per-production). ## Test - The 3 targeted tests pass; both full files green (32 passed). - ruff + pyright clean on the changed files. - The `ask_human`/`AsyncAccepted` → `pending_tool_call_ids` population path remains covered elsewhere (`test_tools.py`, resumable-agent/resume-worker/e2e tests). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated test suite for profile extraction to validate pending tool call attachment workflow. * Refactored pre-check behavior tests to operate on individual extractor configurations. * Adjusted test assertions to align with updated behavior expectations for extraction and dependency tracking. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This was referenced Jun 4, 2026
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
1 participant
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
agent_success_configwith legacyagent_success_configsfirst-entry migration/serializationTesting
PYTEST_ADDOPTS='--no-cov' uv run pytest open_source/reflexio/tests/server/services/agent_success_evaluation/test_agent_success_evaluation_services.py open_source/reflexio/tests/models/test_validators.py open_source/reflexio/tests/server/services/test_base_generation_service.py open_source/reflexio/tests/server/api_endpoints/test_evaluations_regenerate_api.py -qfrom enterprise repo: 203 passeduv run ruff check ...on touched Python filesuv run pyright ...on touched backend filescd open_source/reflexio/docs && npm run lint && npm run build(lint has existing warnings only)Review follow-up
normalize_legacy_config_shape()toconfig_schema.pyand apply it at thelocal_file_config_storageload boundary, so OSS on-disk configs persisted before the single-extractor refactor recover their singular extractor/evaluator fields instead of silently dropping the legacy list keys*_extractor_configsfallback inresume_worker._select_current_extractor_confignow that the deprecated computed list views are gonePYTEST_ADDOPTS='--no-cov' uv run pytest -o 'addopts=' open_source/reflexio/reflexio/server/services/configurator/test_config_storage.py open_source/reflexio/tests/server/services/extraction/test_resume_worker.py(incl. newtest_load_config_upgrades_legacy_list_shape): passeduv run ruff check+uv run pyrightclean on touched filesSummary by CodeRabbit
Refactor
User Experience
Stability