feat: Update environment configuration and enhance logging for local …#60
Merged
feat: Update environment configuration and enhance logging for local …#60
Conversation
…development; normalize Robot Framework CSS selectors
Contributor
WalkthroughAdds a Robot Framework CSS-selector normalizer and integrates it into the workflow pipeline; introduces LOG_FORMAT support and console color detection; adjusts local dev bootstrapping (image tags, HOST_ROBOT_TESTS_DIR resolution, run.sh env exports); and ensures final 100% progress events are suppressed when workflows are unregistered. Changes
Sequence Diagram(s)sequenceDiagram
participant CrewAI as CrewAI
participant Workflow as WorkflowService
participant Normalizer as CSSNormalizer
participant Validator as RobotValidator
participant Executor as DockerExecutor
participant Progress as ProgressQueue
CrewAI->>Workflow: emit generated robot code
Workflow->>Normalizer: submit raw code for normalization
Normalizer-->>Workflow: return normalized robot code
Workflow->>Validator: validate normalized code
Validator-->>Workflow: validation result
Workflow->>Executor: run in container
Executor-->>Workflow: execution results
Workflow->>Progress: push progress events (including final 100%)
Progress-->>Workflow: acknowledge / suppressed if unregistered
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/backend/config/logging_config.py (1)
69-101:⚠️ Potential issue | 🟡 MinorAvoid writing ANSI-colored console logs into
application.log.The same formatter instance is used for both file and console handlers. When
LOG_FORMAT=consoleand stdout is a TTY (line 70:colors=sys.stdout.isatty()), the rotating file handler receives ANSI color escape sequences, pollutinglogs/application.log.Use separate formatters: file formatter with
colors=False, console formatter withcolors=sys.stdout.isatty().🛠️ Proposed fix: separate file and console formatters
- renderer = ( - structlog.dev.ConsoleRenderer(colors=sys.stdout.isatty()) - if os.environ.get("LOG_FORMAT", "").lower() == "console" - else structlog.processors.JSONRenderer() - ) - formatter = structlog.stdlib.ProcessorFormatter( - processor=renderer, - foreign_pre_chain=_SHARED_PROCESSORS, - ) + use_console_format = os.environ.get("LOG_FORMAT", "").lower() == "console" + file_renderer = ( + structlog.dev.ConsoleRenderer(colors=False) + if use_console_format + else structlog.processors.JSONRenderer() + ) + console_renderer = ( + structlog.dev.ConsoleRenderer(colors=sys.stdout.isatty()) + if use_console_format + else structlog.processors.JSONRenderer() + ) + file_formatter = structlog.stdlib.ProcessorFormatter( + processor=file_renderer, + foreign_pre_chain=_SHARED_PROCESSORS, + ) + console_formatter = structlog.stdlib.ProcessorFormatter( + processor=console_renderer, + foreign_pre_chain=_SHARED_PROCESSORS, + ) @@ - file_handler = logging.StreamHandler(sys.stdout) + file_handler = logging.StreamHandler(sys.stdout) - file_handler.setFormatter(formatter) + file_handler.setFormatter( + console_formatter + if isinstance(file_handler, logging.StreamHandler) and file_handler.stream is sys.stdout + else file_formatter + ) @@ console_handler = logging.StreamHandler(sys.stdout) - console_handler.setFormatter(formatter) + console_handler.setFormatter(console_formatter)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/config/logging_config.py` around lines 69 - 101, The current code reuses a single ProcessorFormatter named formatter for both file_handler and console_handler which causes ANSI color escape sequences to be written into application.log when LOG_FORMAT=console; create two separate renderers/formatters: a file_renderer = structlog.dev.ConsoleRenderer(colors=False) (or JSONRenderer when not console) and a console_renderer = structlog.dev.ConsoleRenderer(colors=sys.stdout.isatty()) (or JSONRenderer as appropriate), then build two ProcessorFormatter instances (e.g., file_formatter and console_formatter) using the same _SHARED_PROCESSORS but different renderer arguments, call file_handler.setFormatter(file_formatter) and console_handler.setFormatter(console_formatter), and keep the existing logic that skips adding a separate console_handler when file_handler is a StreamHandler(sys.stdout).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@run.sh`:
- Around line 40-44: The current assignment to SCRIPT_DIR can run pwd (falling
back to the caller dir) when the initial cd fails; change the command grouping
so the fallback to plain pwd only happens if cd into "$(dirname
"${BASH_SOURCE[0]}")" succeeded. In other words, ensure SCRIPT_DIR is computed
by first performing cd, and only then running "pwd -W 2>/dev/null || pwd" within
that successful cd context (so HOST_ROBOT_TESTS_DIR uses the script directory,
not the caller's cwd); reference the SCRIPT_DIR assignment and
HOST_ROBOT_TESTS_DIR and the use of pwd -W in your fix.
In `@src/backend/crew_ai/crew.py`:
- Around line 419-427: The duplicate 100% progress event can be enqueued if
_push_if_forward() checks progress after the workflow was unregistered; update
_push_if_forward in src/backend/crew_ai/progress_events.py to first verify the
workflow is still registered (e.g., confirm the progress_queue or
_current_progress entry exists and matches the given workflow_id) before
comparing/setting progress, so that calls arriving after unregister_workflow()
are ignored and cannot enqueue a stale 100% event; keep the existing dedupe
logic but add this early-stale-guard to prevent re-enqueueing after
unregistration.
In `@src/backend/crew_ai/optimization/nl_feedback_engine.py`:
- Around line 459-488: The RF detection currently only matches ${...} via
_RF_VAR_RE and misses typical Robot Framework table-style lines (keyword +
multiple spaces + args) and locator patterns; update _RF_VAR_RE (used in
_format_feedback_hint) to a broader regex that also matches: (1) lines with
Robot table spacing/columns (e.g., a token followed by 2+ spaces or tabs and
further tokens like "Click `#searchBox`"), (2) common locator prefixes (#, .,
//, css=, xpath=), and (3) assignment patterns like "\w+=\s+Set Variable" or
keywords such as "Fill Text|Click|Input Text|Set Variable". Replace the current
re.compile(r'\$\{[A-Za-z_][A-Za-z0-9_ ]*\}') with a single composite pattern
capturing these cases so lines containing RF keywords/locators/assignments are
appended to code_lines in _format_feedback_hint.
In `@src/backend/crew_ai/robot_code_normalizer.py`:
- Around line 69-73: The current loop that uses _CELL_SPLIT_RE and _BARE_CSS_RE
rewrites any bare `#`/`.` token regardless of context; limit rewriting to actual
locator cells by first determining the cell's role (keyword name, argument
index, or section type) and only apply the css= rewrite when the cell is a
locator argument for locator-taking keywords (e.g., Click, Fill Text, Get Text,
Wait For Elements State) or when in a Variables section with a variable name
containing "locator"/"selector"; update the loop that assigns parts[i] and
increments rewrote to check the keyword/arg position and section metadata before
replacing a bare match so non-locator cells (assertions, tags, messages) are
left unchanged.
---
Outside diff comments:
In `@src/backend/config/logging_config.py`:
- Around line 69-101: The current code reuses a single ProcessorFormatter named
formatter for both file_handler and console_handler which causes ANSI color
escape sequences to be written into application.log when LOG_FORMAT=console;
create two separate renderers/formatters: a file_renderer =
structlog.dev.ConsoleRenderer(colors=False) (or JSONRenderer when not console)
and a console_renderer =
structlog.dev.ConsoleRenderer(colors=sys.stdout.isatty()) (or JSONRenderer as
appropriate), then build two ProcessorFormatter instances (e.g., file_formatter
and console_formatter) using the same _SHARED_PROCESSORS but different renderer
arguments, call file_handler.setFormatter(file_formatter) and
console_handler.setFormatter(console_formatter), and keep the existing logic
that skips adding a separate console_handler when file_handler is a
StreamHandler(sys.stdout).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4c8c570b-fa52-4301-b031-3020c7ac31fa
📒 Files selected for processing (11)
.env.examplerun.shsrc/backend/.env.examplesrc/backend/config/logging_config.pysrc/backend/crew_ai/crew.pysrc/backend/crew_ai/library_context/browser_context.pysrc/backend/crew_ai/optimization/nl_feedback_engine.pysrc/backend/crew_ai/robot_code_normalizer.pysrc/backend/crew_ai/tasks.pysrc/backend/services/workflow_service.pytools/browser_use_service.py
Comment on lines
+40
to
+44
| # When running locally (not inside Docker Compose), HOST_ROBOT_TESTS_DIR must be | ||
| # an absolute Windows path so the Docker daemon can resolve the bind mount. | ||
| # The .env value (./robot_tests) is relative and only valid inside Docker Compose. | ||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -W 2>/dev/null || pwd)" | ||
| export HOST_ROBOT_TESTS_DIR="${SCRIPT_DIR}/robot_tests" |
Contributor
There was a problem hiding this comment.
Make the pwd -W fallback conditional on a successful cd.
Line 43 can fall back to pwd even if the cd fails, which would export HOST_ROBOT_TESTS_DIR from the caller’s current directory instead of the script directory.
🛠️ Proposed fix
-SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -W 2>/dev/null || pwd)"
+if ! SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && { pwd -W 2>/dev/null || pwd; })"; then
+ echo "Error: unable to resolve script directory."
+ exit 1
+fi
export HOST_ROBOT_TESTS_DIR="${SCRIPT_DIR}/robot_tests"🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 43-43: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@run.sh` around lines 40 - 44, The current assignment to SCRIPT_DIR can run
pwd (falling back to the caller dir) when the initial cd fails; change the
command grouping so the fallback to plain pwd only happens if cd into "$(dirname
"${BASH_SOURCE[0]}")" succeeded. In other words, ensure SCRIPT_DIR is computed
by first performing cd, and only then running "pwd -W 2>/dev/null || pwd" within
that successful cd context (so HOST_ROBOT_TESTS_DIR uses the script directory,
not the caller's cwd); reference the SCRIPT_DIR assignment and
HOST_ROBOT_TESTS_DIR and the use of pwd -W in your fix.
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/backend/crew_ai/optimization/nl_feedback_engine.py (1)
460-470: Optional: hoist the static header to a module-level constant.The warning header is a pure constant but gets reconstructed on every call. Making it a module-level string (and docstring the intent there) slightly improves readability and makes it trivially patchable from tests if you ever want a fixture-level override.
♻️ Proposed refactor
+_USER_FEEDBACK_HEADER = ( + "\u26a0\ufe0f USER FEEDBACK (reference context from a past test — " + "do NOT copy any code literally, treat as guidance only):" +) + @@ def _format_feedback_hint(self, feedback_text: str) -> str: - # Pass feedback through verbatim with a universal warning header. - # The header travels with every hint to every agent (Planner, Assembler, - # Validator), so the "do not copy literally" guardrail applies even for - # agents whose task prompts lack an explicit disclaimer. Preserving the - # original line order retains the user's reasoning structure. - return ( - "\u26a0\ufe0f USER FEEDBACK (reference context from a past test — " - "do NOT copy any code literally, treat as guidance only):\n" - f"{feedback_text.strip()}" - ) + # Header travels with every hint so the "do not copy literally" guardrail + # applies to every agent (Planner, Assembler, Validator) regardless of + # their task-prompt disclaimers. + return f"{_USER_FEEDBACK_HEADER}\n{feedback_text.strip()}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/crew_ai/optimization/nl_feedback_engine.py` around lines 460 - 470, Hoist the static warning header string out of _format_feedback_hint into a module-level constant (e.g., FEEDBACK_WARNING_HEADER) with a brief docstring explaining it's the universal "do NOT copy literally" header used for all agent hints; then update _format_feedback_hint to return f"{FEEDBACK_WARNING_HEADER}\n{feedback_text.strip()}" so the header is constructed once and remains easily overrideable in tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/backend/crew_ai/optimization/nl_feedback_engine.py`:
- Around line 460-470: Hoist the static warning header string out of
_format_feedback_hint into a module-level constant (e.g.,
FEEDBACK_WARNING_HEADER) with a brief docstring explaining it's the universal
"do NOT copy literally" header used for all agent hints; then update
_format_feedback_hint to return
f"{FEEDBACK_WARNING_HEADER}\n{feedback_text.strip()}" so the header is
constructed once and remains easily overrideable in tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c526a4c6-6b72-4cdf-9841-1b1a5a6e6739
📒 Files selected for processing (5)
src/backend/crew_ai/optimization/nl_feedback_engine.pysrc/backend/crew_ai/progress_events.pysrc/backend/crew_ai/robot_code_normalizer.pytests/test_crew_ai/test_robot_code_normalizer.pytests/test_optimization/test_nl_feedback_pipeline.py
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.
…development; normalize Robot Framework CSS selectors
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests