Skip to content

Conversation

@parashardhapola
Copy link
Member

@parashardhapola parashardhapola commented Sep 17, 2025

PR Type

Enhancement, Other


Description

  • Add n_parallel_clusters parameter for concurrent model requests

  • Update dependencies and version bump to 0.10.0

  • Improve error handling for empty API responses

  • Add input validation using Pydantic models


Diagram Walkthrough

flowchart LR
  A["User Input"] --> B["Input Validation"]
  B --> C["Submit Job with n_parallel_clusters"]
  C --> D["API Processing"]
  D --> E["Poll Results with Error Handling"]
Loading

File Walkthrough

Relevant files
Miscellaneous
__init__.py
Version bump to 0.10.0                                                                     

cytetype/init.py

  • Bump version from 0.9.2 to 0.10.0
+1/-1     
Error handling
client.py
Improve API error handling                                                             

cytetype/client.py

  • Add error handling for empty API responses
  • Raise CyteTypeAPIError when no response received
+2/-0     
Enhancement
main.py
Add parallel processing and input validation                         

cytetype/main.py

  • Add n_parallel_clusters parameter to run method
  • Add input validation using Pydantic models
  • Import InputData schema for validation
  • Refactor LLM config validation logic
+21/-2   
server_schema.py
Add parallel clusters schema field                                             

cytetype/server_schema.py

  • Add nParallelClusters field to InputData model
  • Set constraints: minimum 1, maximum 50, default 2
  • Update example data with parallel clusters value
+7/-0     
Dependencies
pyproject.toml
Update project dependencies                                                           

pyproject.toml

  • Update anndata dependency from ~0.11.4 to >=0.9.2
  • Update requests from ~2.32.3 to ~2.32.5
  • Update all dev dependencies to latest versions
  • Change mypy Python version from 3.12 to 3.11
+9/-9     

@qodo-code-review qodo-code-review bot changed the title n_parallel_clusters support and deps update n_parallel_clusters support and deps update Sep 17, 2025
@qodo-code-review
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

ValidationError import

The code catches ValidationError but there is no added import for it in the diff. Ensure ValidationError is imported from pydantic to avoid NameError at runtime.

try:
    input_data = InputData(**input_data).model_dump()
except ValidationError as e:
    logger.error(f"Validation error: {e}")
    raise e

if llm_configs:
    try:
        llm_configs = [LLMModelConfig(**x).model_dump() for x in llm_configs]
    except ValidationError as e:
        logger.error(f"Validation error: {e}")
        raise e
Payload construction

The submit_job payload uses a conditional inline for llm_configs that may serialize to a malformed dict; double-check the resulting structure matches the server API (key present with list or omitted) and that empty lists are handled as intended.

job_id = submit_job(
    {
        "input_data": input_data,
        "llm_configs": llm_configs
        if llm_configs
        else None,
    },
Error specificity

On empty raw_response, a generic CyteTypeAPIError "No response from API" is raised; consider including job_id or status to aid debugging and avoid masking transient 'not_found' scenarios.

if not raw_response:
    raise CyteTypeAPIError("No response from API")
current_cluster_status = raw_response.get("clusterStatus", {})

@qodo-code-review
Copy link

qodo-code-review bot commented Sep 17, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Loosening the anndata dependency is risky

The anndata dependency constraint was loosened from ~=0.11.4 to >=0.9.2. This
change is risky due to potential breaking changes in older anndata versions and
could cause compatibility issues.

Examples:

pyproject.toml [8]
    "anndata>=0.9.2",

Solution Walkthrough:

Before:

# pyproject.toml
...
dependencies = [
    "anndata~=0.11.4", # Restricts to versions >=0.11.4 and <0.12.0
    ...
]
...

After:

# pyproject.toml
...
dependencies = [
    "anndata>=0.9.2", # Allows any version from 0.9.2 upwards
    ...
]
...
Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies a significant risk in loosening the anndata dependency, which could break the package for users with older versions and is a critical stability concern.

Medium
General
Remove redundant exception handling blocks

Remove the redundant try...except blocks that catch, log, and then re-raise
ValidationError, and instead allow the exception to propagate up the call stack.

cytetype/main.py [315-328]

-try:
-    input_data = InputData(**input_data).model_dump()
-except ValidationError as e:
-    logger.error(f"Validation error: {e}")
-    raise e
+# Let Pydantic's ValidationError propagate naturally.
+input_data = InputData(**input_data).model_dump()
 
 if llm_configs:
-    try:
-        llm_configs = [LLMModelConfig(**x).model_dump() for x in llm_configs]
-    except ValidationError as e:
-        logger.error(f"Validation error: {e}")
-        raise e
+    llm_configs = [LLMModelConfig(**x).model_dump() for x in llm_configs]
 else:
     llm_configs = []
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies a "log and re-raise" anti-pattern, and removing it simplifies the code and improves the error handling strategy.

Low
  • Update

@parashardhapola parashardhapola merged commit 74c6390 into master Sep 17, 2025
1 check passed
@parashardhapola parashardhapola deleted the misc branch November 20, 2025 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants