Fix NaN comparison

[NolanTrem]

Important

Fixes NaN comparison in create_providers method of factory.py to ensure consistent embedding dimensions.

• Behavior:
  • Fixes NaN comparison in create_providers method of factory.py.
  • Ensures embedding.base_dimension and completion_embedding.base_dimension are either both NaN or equal.
• Imports:
  • Adds import math to handle NaN comparisons.

^{This description was created by for e0671c8. It will automatically update as commits are pushed.}

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to e0671c8 in 1 minute and 36 seconds

More details

• Looked at 24 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 3 drafted comments based on config settings.

1. py/core/main/assembly/factory.py:315

• Draft comment:
  Using math.isnan on base_dimension assumes it's a float. Ensure type safety; if base_dimension could be an int or None, this might raise a TypeError.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
  This is a valid concern - math.isnan() is specifically for float values and will raise TypeError if called on int or None. However, I don't have enough context about the EmbeddingConfig class to know if base_dimension is guaranteed to be a float. The fact that the code was changed to add isnan checks suggests the author knows the type. Without seeing the config class definition, I can't be 100% sure this is an issue.
  I don't have visibility into the EmbeddingConfig class definition, so I can't verify if base_dimension is actually typed as something other than float. The author may have good reason to know it's always a float.
  While the type concern is theoretically valid, without being able to confirm the actual type of base_dimension, this comment risks being incorrect or unnecessary if base_dimension is already guaranteed to be a float.
  Given the uncertainty about the underlying types and lack of strong evidence that this is definitely an issue, we should err on the side of removing this comment.

2. py/core/main/assembly/factory.py:315

• Draft comment:
  Consider extracting the math.isnan() results into well-named variables for clarity, e.g. isEmbeddingNaN and isCompletionNaN. This would simplify the condition and improve readability.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

3. py/core/main/assembly/factory.py:315

• Draft comment:
  The updated condition correctly handles cases where one dimension is NaN and the other is not, and allows both to be NaN. This fix addresses the NaN comparison issue.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

Workflow ID: wflow_uRonrTwrtQ4sVDMB

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

For readability, consider caching the is-NaN checks in variables instead of repeating math.isnan calls.