Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds an agentic, LLM-driven adaptive decision flow to example pipelines, introduces a new agent utility module and public exports, updates an existing use case to use AI-based criteria, and adjusts project metadata/dependencies. Includes a new agentic example, documentation, and concurrency-backed orchestration with optional child pipeline spawning. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant Manager as PipelineManager
participant Pipe as Pipeline (Dummy/ProteinBinding)
participant Utils as provide_llm_context
participant Agent as llm_agent (AgentObserver)
participant LLM as LLM Backend
User->>Manager: start(pipeline setups)
Manager->>Pipe: run()
Pipe->>Pipe: compute scores/trend
Pipe->>Utils: provide_llm_context(context)
Utils-->>Pipe: llm_context (string)
Pipe->>Agent: prompt(message=llm_context)
Agent->>LLM: prompt(sys+message, schema=Schema)
LLM-->>Agent: parsed_response.spawn_new_pipeline
Agent-->>Pipe: response (with parsed decision)
alt spawn_new_pipeline && within limits
Pipe->>Manager: submit child pipeline
Manager-->>Pipe: ack
else No spawn or limit reached
Pipe-->>Manager: continue/finish
end
Manager-->>User: results (approved/rejected counts)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (4)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/protien_binding_usecase/run_protein_binding.py (2)
62-76: Return annotation and docstring are staleFunction doesn’t return a config; it mutates pipeline state and submits a request. Update typing and docstring.
-async def adaptive_decision(pipeline: ProteinBindingPipeline) -> Optional[Dict[str, Any]]: +async def adaptive_decision(pipeline: ProteinBindingPipeline) -> None: @@ - Returns: - Pipeline configuration dictionary for new child pipeline if needed, - None otherwise + Returns: + None
144-145: Pass requiredsub_iter_seqstofinalize
Change at examples/protien_binding_usecase/run_protein_binding.py line 144:- pipeline.finalize() + pipeline.finalize(sub_iter_seqs)
ProteinBindingPipeline.finalize(self, sub_iter_seqs)requires thesub_iter_seqsargument (see src/impress/pipelines/protein_binding.py#L156).
🧹 Nitpick comments (11)
examples/dummy_adaptive.py (1)
52-57: Replace placeholder docstring with actionable context.Add a concise docstring that states inputs/outputs and what the AI gate will decide.
- """ - - CONTEXT OF THE PIPELINE: - - - - """ + """ + Adaptive decision gate for spawning a child pipeline. + Returns early when: + - generation >= max_generations, or + - the AI gate advises not to spawn. + Expects to build an LLM context from the current pipeline state. + """pyproject.toml (1)
5-12: Add license metadata.Include license to avoid packaging warnings and clarify distribution terms.
[project] name = "impress" version = "0.1.0" description = "IMPRESS Protein Binding and Prediction Framework for HPC" authors = [ {name = "Aymen Alsaadi"}, {name = "Javier D. Segura"} ] +license = { file = "LICENSE" }Verify a LICENSE file exists at repo root.
src/impress/utils/agentic/agent.py (2)
4-4: Drop unused import.
Fieldis not used.-from pydantic import BaseModel, Field +from pydantic import BaseModel
83-85: Avoid import-time side effects; lazily initialize the agent.Global instantiation can fail if creds/env aren’t set during import; prefer a lazy singleton.
-llm_agent = AgentObserver() +_LLM_AGENT: AgentObserver | None = None + +def llm_agent() -> AgentObserver: + global _LLM_AGENT + if _LLM_AGENT is None: + _LLM_AGENT = AgentObserver() + return _LLM_AGENTNote: update call sites to use
llm_agent().prompt(...).src/impress/utils/__init__.py (1)
1-1: Export symbols explicitly to satisfy linters.Add all so re-exports are intentional and F401 is silenced.
-from .agentic import llm_agent, provide_llm_context, PipelineContext +from .agentic import llm_agent, provide_llm_context, PipelineContext + +__all__ = ["llm_agent", "provide_llm_context", "PipelineContext"]examples/protien_binding_usecase/run_protein_binding.py (5)
49-49: Avoid hard-coded 3; use the constantUse MAX_SUB_PIPELINES for consistency with the spawning logic.
- max_sub_pipelines=3, # Hardcoded value + max_sub_pipelines=MAX_SUB_PIPELINES,
78-79: Duplicate initialization of sub_iter_seqsRemove the first initialization; it’s redefined at Line 99.
- sub_iter_seqs: Dict[str, str] = {}Also applies to: 99-100
80-91: CSV parsing and robustnessManual split is brittle; use csv.reader and handle bad/missing values cleanly.
+ import csv # Read current scores from CSV file_name = f'af_stats_{pipeline.name}_pass_{pipeline.passes}.csv' - with open(file_name) as fd: - for line in fd.readlines()[1:]: - line = line.strip() - if not line: - continue - - name, *_, score_str = line.split(',') - protein = name.split('.')[0] - pipeline.current_scores[protein] = float(score_str) + with open(file_name, newline="") as fd: + reader = csv.reader(fd) + next(reader, None) # skip header + for row in reader: + if not row: + continue + name = row[0].strip() + score_str = row[-1].strip() + try: + protein = name.split('.')[0] + pipeline.current_scores[protein] = float(score_str) + except ValueError: + logger.error(f"Invalid score for {name}: {score_str}")
104-110: Log exceptions with context and stacktraceUse logger.exception and include protein/pipeline for traceability.
- except Exception as e: - logger.error(e) + except Exception: + logger.exception(f"adaptive_criteria failed for protein={protein} pipeline={pipeline.name}") continue
175-177: Rename the misspelled counter attribute for clarityThe agent class currently defines and uses
pipelines_aproved(lines 65 and 78) alongsidepipelines_rejected(lines 64 and 80) insrc/impress/utils/agentic/agent.py, so the runtime references inrun_protein_binding.pywon’t crash. However, “approved” is misspelled. To avoid confusion, renamepipelines_aproved→pipelines_approvedand update all its usages (including the debug log).examples/agentic/dummy_adaptive.py (1)
132-134: Simplify multi-line debug stringAvoid backslash continuations inside f-strings.
- logger.debug(f"AGENTIC RESULTS: \ - pipelines approved: {llm_agent.pipelines_aproved} \ - pipelines rejected: {llm_agent.pipelines_rejected}") + logger.debug( + f"AGENTIC RESULTS: pipelines approved: {llm_agent.pipelines_aproved} " + f"pipelines rejected: {llm_agent.pipelines_rejected}" + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
docs/.DS_Storeis excluded by!**/.DS_Storeexamples/.DS_Storeis excluded by!**/.DS_Storesrc/.DS_Storeis excluded by!**/.DS_Store
📒 Files selected for processing (10)
examples/agentic/dummy_adaptive.py(1 hunks)examples/agentic/explanation.md(1 hunks)examples/dummy_adaptive.py(2 hunks)examples/protien_binding_usecase/run_protein_binding.py(3 hunks)pyproject.toml(1 hunks)setup.py(0 hunks)src/impress/__init__.py(1 hunks)src/impress/utils/__init__.py(1 hunks)src/impress/utils/agentic/__init__.py(1 hunks)src/impress/utils/agentic/agent.py(1 hunks)
💤 Files with no reviewable changes (1)
- setup.py
🧰 Additional context used
🧬 Code graph analysis (5)
src/impress/utils/__init__.py (1)
src/impress/utils/agentic/agent.py (2)
provide_llm_context(86-92)PipelineContext(11-21)
src/impress/__init__.py (1)
src/impress/utils/agentic/agent.py (2)
provide_llm_context(86-92)PipelineContext(11-21)
src/impress/utils/agentic/__init__.py (1)
src/impress/utils/agentic/agent.py (2)
provide_llm_context(86-92)PipelineContext(11-21)
examples/protien_binding_usecase/run_protein_binding.py (3)
src/impress/utils/agentic/agent.py (3)
provide_llm_context(86-92)PipelineContext(11-21)prompt(74-81)src/impress/pipelines/protein_binding.py (1)
ProteinBindingPipeline(13-231)src/impress/utils/logger.py (2)
error(102-104)debug(90-92)
examples/agentic/dummy_adaptive.py (5)
src/impress/pipelines/setup.py (1)
PipelineSetup(6-55)src/impress/pipelines/impress_pipeline.py (4)
ImpressBasePipeline(7-108)auto_register_task(50-65)run_adaptive_step(67-75)submit_child_pipeline_request(23-31)src/impress/impress_manager.py (2)
ImpressManager(11-211)start(102-211)src/impress/utils/agentic/agent.py (2)
provide_llm_context(86-92)prompt(74-81)examples/protien_binding_usecase/run_protein_binding.py (2)
adaptive_criteria(18-59)adaptive_decision(62-147)
🪛 LanguageTool
examples/agentic/explanation.md
[grammar] ~4-~4: There might be a mistake here.
Context: ...method ### 1st Prototype (Quick & Dirty) - Currently: Randomly stop critieria - Pro...
(QB_NEW_EN)
[grammar] ~5-~5: Ensure spelling is correct
Context: ...ick & Dirty) - Currently: Randomly stop critieria - Proposed approach: provide the model wit...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~6-~6: There might be a mistake here.
Context: ... - Proposed approach: provide the model with following context, the make a decision:...
(QB_NEW_EN)
[grammar] ~6-~6: Ensure spelling is correct
Context: ...ovide the model with following context, the make a decision: - Context (some bas...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~6-~6: ‘make a decision’ might be wordy. Consider a shorter alternative.
Context: ...e the model with following context, the make a decision: - Context (some basic attributes fr...
(EN_WORDINESS_PREMIUM_MAKE_A_DECISION)
[grammar] ~6-~6: There might be a mistake here.
Context: ... following context, the make a decision: - Context (some basic attributes from the ...
(QB_NEW_EN)
[grammar] ~7-~7: There might be a mistake here.
Context: ...c attributes from the Pipelineobject): - Prior score - Current score ...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ... Pipelineobject): - Prior score - Current score - Geneartion number ...
(QB_NEW_EN)
[grammar] ~10-~10: Ensure spelling is correct
Context: ...ior score - Current score - Geneartion number ### 2nd Prototype (Prototype 1....
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~14-~14: There might be a mistake here.
Context: ...via adding more attributes. - Feats: - Consider adding attributes from other re...
(QB_NEW_EN)
[grammar] ~16-~16: Ensure spelling is correct
Context: ...rove dev prompt by having a more robust criteraia
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.17.2)
examples/agentic/explanation.md
7-7: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
8-8: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
9-9: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
10-10: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
🪛 Ruff (0.12.2)
src/impress/utils/__init__.py
1-1: .agentic.llm_agent imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
1-1: .agentic.provide_llm_context imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
1-1: .agentic.PipelineContext imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
src/impress/__init__.py
4-4: impress.utils.llm_agent imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
4-4: impress.utils.provide_llm_context imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
4-4: impress.utils.PipelineContext imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
src/impress/utils/agentic/__init__.py
1-1: .agent.llm_agent imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
1-1: .agent.provide_llm_context imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
1-1: .agent.PipelineContext imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
examples/dummy_adaptive.py
11-11: agnostic_agent.LLMAgent imported but unused
Remove unused import: agnostic_agent.LLMAgent
(F401)
src/impress/utils/agentic/agent.py
4-4: pydantic.Field imported but unused
Remove unused import: pydantic.Field
(F401)
examples/agentic/dummy_adaptive.py
25-25: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
35-35: f-string without any placeholders
Remove extraneous f prefix
(F541)
39-39: f-string without any placeholders
Remove extraneous f prefix
(F541)
43-43: f-string without any placeholders
Remove extraneous f prefix
(F541)
84-84: Local variable trend is assigned to but never used
Remove assignment to unused variable trend
(F841)
🔇 Additional comments (2)
src/impress/utils/agentic/agent.py (1)
67-73: Keep init but attach after counter fields.No functional change; just ensure construction order after moving counters.
examples/agentic/dummy_adaptive.py (1)
123-124: Keep await on ConcurrentExecutionBackend
Initialization returns an awaitable per official documentation, so theawaitis required (libraries.io).Likely an incorrect or invalid review comment.
| def __init__(self, name: str, flow: Any, configs: Dict[str, Any] = {}, **kwargs): | ||
| self.iter_seqs: str = 'MKFLVLACGT' | ||
| self.generation: int = configs.get('generation', 1) | ||
| self.parent_name: str = configs.get('parent_name', 'root') | ||
| self.max_generations: int = configs.get('max_generations', 3) | ||
| super().__init__(name, flow, **configs, **kwargs) |
There was a problem hiding this comment.
Mutable default argument (B006)
configs={} is shared across instances; use None and initialize inside.
-class DummyProteinPipeline(ImpressBasePipeline):
- def __init__(self, name: str, flow: Any, configs: Dict[str, Any] = {}, **kwargs):
+class DummyProteinPipeline(ImpressBasePipeline):
+ def __init__(self, name: str, flow: Any, configs: Dict[str, Any] | None = None, **kwargs):
+ if configs is None:
+ configs = {}
self.iter_seqs: str = 'MKFLVLACGT'
self.generation: int = configs.get('generation', 1)📝 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.
| def __init__(self, name: str, flow: Any, configs: Dict[str, Any] = {}, **kwargs): | |
| self.iter_seqs: str = 'MKFLVLACGT' | |
| self.generation: int = configs.get('generation', 1) | |
| self.parent_name: str = configs.get('parent_name', 'root') | |
| self.max_generations: int = configs.get('max_generations', 3) | |
| super().__init__(name, flow, **configs, **kwargs) | |
| def __init__(self, name: str, flow: Any, configs: Dict[str, Any] | None = None, **kwargs): | |
| if configs is None: | |
| configs = {} | |
| self.iter_seqs: str = 'MKFLVLACGT' | |
| self.generation: int = configs.get('generation', 1) | |
| self.parent_name: str = configs.get('parent_name', 'root') | |
| self.max_generations: int = configs.get('max_generations', 3) | |
| super().__init__(name, flow, **configs, **kwargs) |
🧰 Tools
🪛 Ruff (0.12.2)
25-25: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
🤖 Prompt for AI Agents
In examples/agentic/dummy_adaptive.py around lines 25 to 30, the __init__ uses a
mutable default argument configs={} which is shared between instances; change
the signature to accept configs: Optional[Dict[str, Any]] = None (import
Optional) and inside the method set configs = {} if configs is None, then
proceed to read configs.get(...) and call super().__init__(name, flow,
**configs, **kwargs) so each instance gets its own configs dict.
| return f"/bin/echo 'Analyzing' && /bin/date" | ||
|
|
||
| @self.auto_register_task() | ||
| async def fitness_evaluation(*args, **kwargs) -> str: | ||
| return f"/bin/echo 'Evaluating' && /bin/date" | ||
|
|
||
| @self.auto_register_task() | ||
| async def optimization_step(*args, **kwargs) -> str: | ||
| return f"/bin/echo 'Optimizing' && /bin/date" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove f-prefix from static strings (F541)
No expressions in these f-strings.
- return f"/bin/echo 'Analyzing' && /bin/date"
+ return "/bin/echo 'Analyzing' && /bin/date"
@@
- return f"/bin/echo 'Evaluating' && /bin/date"
+ return "/bin/echo 'Evaluating' && /bin/date"
@@
- return f"/bin/echo 'Optimizing' && /bin/date"
+ return "/bin/echo 'Optimizing' && /bin/date"📝 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.
| return f"/bin/echo 'Analyzing' && /bin/date" | |
| @self.auto_register_task() | |
| async def fitness_evaluation(*args, **kwargs) -> str: | |
| return f"/bin/echo 'Evaluating' && /bin/date" | |
| @self.auto_register_task() | |
| async def optimization_step(*args, **kwargs) -> str: | |
| return f"/bin/echo 'Optimizing' && /bin/date" | |
| return "/bin/echo 'Analyzing' && /bin/date" | |
| @self.auto_register_task() | |
| async def fitness_evaluation(*args, **kwargs) -> str: | |
| return "/bin/echo 'Evaluating' && /bin/date" | |
| @self.auto_register_task() | |
| async def optimization_step(*args, **kwargs) -> str: | |
| return "/bin/echo 'Optimizing' && /bin/date" |
🧰 Tools
🪛 Ruff (0.12.2)
35-35: f-string without any placeholders
Remove extraneous f prefix
(F541)
39-39: f-string without any placeholders
Remove extraneous f prefix
(F541)
43-43: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
In examples/agentic/dummy_adaptive.py around lines 35 to 44 the return
statements use f-strings with no interpolations (e.g. return f"/bin/echo
'Analyzing' && /bin/date"), triggering F541; remove the unnecessary f-prefix on
each static string return (change to normal double-quoted or single-quoted
string literals) for fitness_evaluation, optimization_step, and the earlier
analyzer task so the strings are plain literals without f.
| if percent_change > 2: | ||
| trend = "improving" | ||
| elif percent_change < -2: | ||
| trend = "degrading" | ||
| else: | ||
| trend = "stable" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use the computed trend in context (fix F841)
Either remove unused vars or include them in the model/context so the LLM sees them. Suggest adding fields to PipelineContextDummy and passing them through.
class PipelineContextDummy(BaseModel):
previous_score: float
current_score: float
generation: int
+ trend: str
+ avg_score_change: float
@@
- context = PipelineContextDummy(
+ context = PipelineContextDummy(
previous_score=previous_score,
current_score=current_score,
- generation=pipeline.generation,
+ generation=pipeline.generation,
+ trend=trend,
+ avg_score_change=percent_change,
)Also applies to: 19-23, 86-91
🧰 Tools
🪛 Ruff (0.12.2)
84-84: Local variable trend is assigned to but never used
Remove assignment to unused variable trend
(F841)
| # Adaptive Agentic, IMPRESS use case | ||
|
|
||
| ## Objective: Inject Agentic AI in the `adaptive_decision` method | ||
| ### 1st Prototype (Quick & Dirty) | ||
| - Currently: Randomly stop critieria | ||
| - Proposed approach: provide the model with following context, the make a decision: | ||
| - Context (some basic attributes from the `Pipeline`object): | ||
| - Prior score | ||
| - Current score | ||
| - Geneartion number | ||
|
|
||
| ### 2nd Prototype (Prototype 1.0) | ||
| - Proposed approach: provide the model with more context via adding more attributes. | ||
| - Feats: | ||
| - Consider adding attributes from other related pipelines (e.g., parent) | ||
| - Improve dev prompt by having a more robust criteraia No newline at end of file |
There was a problem hiding this comment.
Fix typos and Markdown list indentation.
Several misspellings (“critieria”, “Geneartion”), grammar issues, and MD007 list indentation errors.
-# Adaptive Agentic, IMPRESS use case
+# Adaptive, Agentic IMPRESS use case
@@
-### 1st Prototype (Quick & Dirty)
-- Currently: Randomly stop critieria
-- Proposed approach: provide the model with following context, the make a decision:
- - Context (some basic attributes from the `Pipeline`object):
- - Prior score
- - Current score
- - Geneartion number
+### 1st Prototype (Quick & Dirty)
+- Currently: Random stop criteria
+- Proposed approach: provide the model with the following context, then decide:
+ - Context (basic attributes from the `Pipeline` object):
+ - Prior score
+ - Current score
+ - Generation number
@@
-### 2nd Prototype (Prototype 1.0)
-- Proposed approach: provide the model with more context via adding more attributes.
- - Feats:
- - Consider adding attributes from other related pipelines (e.g., parent)
- - Improve dev prompt by having a more robust criteraia
+### 2nd Prototype (Prototype 1.0)
+- Proposed approach: provide the model with more context by adding attributes.
+ - Features:
+ - Consider attributes from related pipelines (e.g., parent)
+ - Improve the prompt with more robust criteria📝 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.
| # Adaptive Agentic, IMPRESS use case | |
| ## Objective: Inject Agentic AI in the `adaptive_decision` method | |
| ### 1st Prototype (Quick & Dirty) | |
| - Currently: Randomly stop critieria | |
| - Proposed approach: provide the model with following context, the make a decision: | |
| - Context (some basic attributes from the `Pipeline`object): | |
| - Prior score | |
| - Current score | |
| - Geneartion number | |
| ### 2nd Prototype (Prototype 1.0) | |
| - Proposed approach: provide the model with more context via adding more attributes. | |
| - Feats: | |
| - Consider adding attributes from other related pipelines (e.g., parent) | |
| - Improve dev prompt by having a more robust criteraia | |
| # Adaptive, Agentic IMPRESS use case | |
| ## Objective: Inject Agentic AI in the `adaptive_decision` method | |
| ### 1st Prototype (Quick & Dirty) | |
| - Currently: Random stop criteria | |
| - Proposed approach: provide the model with the following context, then decide: | |
| - Context (basic attributes from the `Pipeline` object): | |
| - Prior score | |
| - Current score | |
| - Generation number | |
| ### 2nd Prototype (Prototype 1.0) | |
| - Proposed approach: provide the model with more context by adding attributes. | |
| - Features: | |
| - Consider attributes from related pipelines (e.g., parent) | |
| - Improve the prompt with more robust criteria |
🧰 Tools
🪛 LanguageTool
[grammar] ~4-~4: There might be a mistake here.
Context: ...method ### 1st Prototype (Quick & Dirty) - Currently: Randomly stop critieria - Pro...
(QB_NEW_EN)
[grammar] ~5-~5: Ensure spelling is correct
Context: ...ick & Dirty) - Currently: Randomly stop critieria - Proposed approach: provide the model wit...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~6-~6: There might be a mistake here.
Context: ... - Proposed approach: provide the model with following context, the make a decision:...
(QB_NEW_EN)
[grammar] ~6-~6: Ensure spelling is correct
Context: ...ovide the model with following context, the make a decision: - Context (some bas...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~6-~6: ‘make a decision’ might be wordy. Consider a shorter alternative.
Context: ...e the model with following context, the make a decision: - Context (some basic attributes fr...
(EN_WORDINESS_PREMIUM_MAKE_A_DECISION)
[grammar] ~6-~6: There might be a mistake here.
Context: ... following context, the make a decision: - Context (some basic attributes from the ...
(QB_NEW_EN)
[grammar] ~7-~7: There might be a mistake here.
Context: ...c attributes from the Pipelineobject): - Prior score - Current score ...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ... Pipelineobject): - Prior score - Current score - Geneartion number ...
(QB_NEW_EN)
[grammar] ~10-~10: Ensure spelling is correct
Context: ...ior score - Current score - Geneartion number ### 2nd Prototype (Prototype 1....
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~14-~14: There might be a mistake here.
Context: ...via adding more attributes. - Feats: - Consider adding attributes from other re...
(QB_NEW_EN)
[grammar] ~16-~16: Ensure spelling is correct
Context: ...rove dev prompt by having a more robust criteraia
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.17.2)
7-7: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
8-8: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
9-9: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
10-10: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
| from concurrent.futures import ThreadPoolExecutor | ||
| from radical.asyncflow import ConcurrentExecutionBackend | ||
|
|
||
| from agnostic_agent import LLMAgent |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused LLMAgent import; import the public API instead.
Use the re-exported symbols to keep examples aligned with the package surface and silence F401.
-from agnostic_agent import LLMAgent
+from impress import llm_agent, provide_llm_context, PipelineContext📝 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.
| from agnostic_agent import LLMAgent | |
| from impress import llm_agent, provide_llm_context, PipelineContext |
🧰 Tools
🪛 Ruff (0.12.2)
11-11: agnostic_agent.LLMAgent imported but unused
Remove unused import: agnostic_agent.LLMAgent
(F401)
🤖 Prompt for AI Agents
In examples/dummy_adaptive.py around line 11, the file imports LLMAgent directly
which is unused and triggers an F401; replace that import with the package's
public API re-export (import the symbol from agnostic_agent public surface) or
remove the unused import entirely so the example uses the re-exported symbols
and aligns with the package surface, ensuring no unused imports remain.
src/impress/__init__.py
Outdated
| "ImpressBasePipeline", | ||
| "PipelineSetup", | ||
| ] | ||
| from impress.utils import llm_agent, provide_llm_context, PipelineContext No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add all to expose re-exports and fix F401
Top-level re-exports are fine; declare all so the imports are “used” and the public surface is explicit.
from impress.utils import llm_agent, provide_llm_context, PipelineContext
+
+__all__ = (
+ "PipelineSetup",
+ "ImpressManager",
+ "ImpressBasePipeline",
+ "llm_agent",
+ "provide_llm_context",
+ "PipelineContext",
+)📝 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.
| from impress.utils import llm_agent, provide_llm_context, PipelineContext | |
| from impress.utils import llm_agent, provide_llm_context, PipelineContext | |
| __all__ = ( | |
| "PipelineSetup", | |
| "ImpressManager", | |
| "ImpressBasePipeline", | |
| "llm_agent", | |
| "provide_llm_context", | |
| "PipelineContext", | |
| ) |
🧰 Tools
🪛 Ruff (0.12.2)
4-4: impress.utils.llm_agent imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
4-4: impress.utils.provide_llm_context imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
4-4: impress.utils.PipelineContext imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
🤖 Prompt for AI Agents
In src/impress/__init__.py around line 4, the top-level re-exports llm_agent,
provide_llm_context, and PipelineContext are imported but trigger F401; add an
explicit __all__ = ["llm_agent", "provide_llm_context", "PipelineContext"] to
declare the public API and mark those imports as used, keeping the re-exports
intact and avoiding the unused-import lint error.
| @@ -0,0 +1 @@ | |||
| from .agent import llm_agent, provide_llm_context, PipelineContext | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Export via all to silence F401 and define public API
Re-export purpose is correct; add all and drop trailing space to satisfy Ruff and make intent explicit.
-from .agent import llm_agent, provide_llm_context, PipelineContext
+from .agent import llm_agent, provide_llm_context, PipelineContext
+
+__all__ = (
+ "llm_agent",
+ "provide_llm_context",
+ "PipelineContext",
+)📝 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.
| from .agent import llm_agent, provide_llm_context, PipelineContext | |
| from .agent import llm_agent, provide_llm_context, PipelineContext | |
| __all__ = ( | |
| "llm_agent", | |
| "provide_llm_context", | |
| "PipelineContext", | |
| ) |
🧰 Tools
🪛 Ruff (0.12.2)
1-1: .agent.llm_agent imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
1-1: .agent.provide_llm_context imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
1-1: .agent.PipelineContext imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
🤖 Prompt for AI Agents
In src/impress/utils/agentic/__init__.py around line 1, the module re-exports
llm_agent, provide_llm_context, and PipelineContext but lacks an explicit
__all__ and has a trailing space in the import; add an __all__ = ["llm_agent",
"provide_llm_context", "PipelineContext"] immediately after the import and
remove the trailing space in the from .agent import line to silence F401 and
make the public API explicit.
| pipelines_rejected = 0 | ||
| pipelines_aproved = 0 | ||
| def __init__(self) -> None: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make counters instance fields and fix spelling.
Class vars here are easy to shadow; also “aproved” is misspelled.
-class AgentObserver():
- pipelines_rejected = 0
- pipelines_aproved = 0
+class AgentObserver:
+ def __init__(self) -> None:
+ self.pipelines_rejected: int = 0
+ self.pipelines_approved: int = 0
+ # initialized below...Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/impress/utils/agentic/agent.py around lines 64 to 66, the counters are
declared as class variables and one is misspelled; move pipelines_rejected and
pipelines_approved to be instance attributes by removing the class-level
declarations and initializing self.pipelines_rejected = 0 and
self.pipelines_approved = 0 inside the __init__ method, rename the misspelled
"aproved" to "approved", and update any references in the class to use
self.pipelines_rejected / self.pipelines_approved.
| response = await self.agent_.prompt(*args, **kwargs) | ||
| spawn_new_piepline = response.parsed_response.spawn_new_pipeline | ||
| if spawn_new_piepline: | ||
| self.pipelines_aproved += 1 | ||
| else: | ||
| self.pipelines_rejected += 1 | ||
| return response |
There was a problem hiding this comment.
Fix typo and harden parsing on agent response.
Correct spawn_new_piepline, update counter name, and guard for missing parsed_response.
- async def prompt(self, *args, **kwargs):
- response = await self.agent_.prompt(*args, **kwargs)
- spawn_new_piepline = response.parsed_response.spawn_new_pipeline
- if spawn_new_piepline:
- self.pipelines_aproved += 1
- else:
- self.pipelines_rejected += 1
- return response
+ async def prompt(self, *args, **kwargs):
+ response = await self.agent_.prompt(*args, **kwargs)
+ try:
+ decision = bool(response.parsed_response.spawn_new_pipeline)
+ except Exception:
+ decision = False
+ if decision:
+ self.pipelines_approved += 1
+ else:
+ self.pipelines_rejected += 1
+ return response📝 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.
| response = await self.agent_.prompt(*args, **kwargs) | |
| spawn_new_piepline = response.parsed_response.spawn_new_pipeline | |
| if spawn_new_piepline: | |
| self.pipelines_aproved += 1 | |
| else: | |
| self.pipelines_rejected += 1 | |
| return response | |
| async def prompt(self, *args, **kwargs): | |
| response = await self.agent_.prompt(*args, **kwargs) | |
| try: | |
| decision = bool(response.parsed_response.spawn_new_pipeline) | |
| except Exception: | |
| decision = False | |
| if decision: | |
| self.pipelines_approved += 1 | |
| else: | |
| self.pipelines_rejected += 1 | |
| return response |
🤖 Prompt for AI Agents
In src/impress/utils/agentic/agent.py around lines 75 to 81, fix the typo and
harden parsing of the agent response: correct the misspelled attribute name
spawn_new_piepline to spawn_new_pipeline, rename pipelines_aproved to
pipelines_approved, and guard against missing parsed_response or missing
attribute by checking if response and getattr(response, "parsed_response", None)
exist then use getattr(response.parsed_response, "spawn_new_pipeline", False)
(treat missing as False) before incrementing the appropriate counter; ensure the
counters are initialized and increment atomically (e.g., += 1) only when the
boolean is True/False as determined.
| def provide_llm_context(pipeline_context: PipelineContext) -> dict: | ||
| pipeline_field_values = pipeline_context.model_dump_json() | ||
|
|
||
| llm_context = f""" | ||
| This is the context of the current pipeline: {pipeline_field_values} | ||
| """ | ||
| return llm_context | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix return annotation and trim indentation in context builder.
The function returns a string; annotate accordingly. Optionally dedent to avoid leading spaces.
-def provide_llm_context(pipeline_context: PipelineContext) -> dict:
+def provide_llm_context(pipeline_context: PipelineContext) -> str:
pipeline_field_values = pipeline_context.model_dump_json()
- llm_context = f"""
- This is the context of the current pipeline: {pipeline_field_values}
- """
+ llm_context = f"This is the context of the current pipeline: {pipeline_field_values}"
return llm_context📝 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.
| def provide_llm_context(pipeline_context: PipelineContext) -> dict: | |
| pipeline_field_values = pipeline_context.model_dump_json() | |
| llm_context = f""" | |
| This is the context of the current pipeline: {pipeline_field_values} | |
| """ | |
| return llm_context | |
| def provide_llm_context(pipeline_context: PipelineContext) -> str: | |
| pipeline_field_values = pipeline_context.model_dump_json() | |
| llm_context = f"This is the context of the current pipeline: {pipeline_field_values}" | |
| return llm_context |
🤖 Prompt for AI Agents
In src/impress/utils/agentic/agent.py around lines 86 to 93, the function
provide_llm_context is annotated to return a dict but actually returns a string
and the triple-quoted f-string includes unwanted leading indentation; change the
return type annotation to str and produce a trimmed/undented string (e.g., use
textwrap.dedent or strip on the constructed string or build it without extra
indentation) so the returned context has no leading spaces.
1 participant
Summary by CodeRabbit
New Features
Documentation
Chores