fix(optimise-framework-schema): move framework feature fields to JSON… - #1249
Conversation
|
Connected to Huly®: UNOPL-1395 |
|
Claude finished @JayGhiya's task —— View job PR Review: Framework Schema OptimizationTodo List:
✅ Overall AssessmentThis is a well-executed schema refactoring that moves framework feature fields into a JSONB payload. The changes maintain backward compatibility through properties while enabling more flexible schema evolution. 🔍 Code Quality & Best PracticesStrengths:
Minor improvements suggested:
🐛 Potential IssuesCritical - Test Compatibility: FrameworkFeature(
language=language,
library=library_name,
feature_key=feature_key,
description=feature_data.get("description"), # ❌ No longer a direct field
target_level=feature_data.get("target_level"), # ❌ No longer a direct field
concept=feature_data.get("concept"), # ❌ No longer a direct field
# ... other removed fields
)This will cause test failures because these fields are now stored in the Moderate Issues:
⚡ Performance ConsiderationsPositive:
Watch for:
🔒 SecurityNo significant concerns identified:
🧪 Test CoverageAction Required: # Current (broken):
FrameworkFeature(
description=feature_data.get("description"),
target_level=feature_data.get("target_level", "function"),
# ...
)
# Should be:
FrameworkFeature(
language=language,
library=library_name,
feature_key=feature_key,
feature_definition={
"description": feature_data.get("description"),
"target_level": feature_data.get("target_level", "function"),
"concept": feature_data.get("concept", "AnnotationLike"),
"locator_strategy": feature_data.get("locator_strategy", "VariableBound"),
"construct_query": feature_data.get("construct_query"),
"startpoint": feature_data.get("startpoint", False),
}
)Test files that likely need attention:
🚀 Recommendations
📊 Schema Migration ImpactThis change represents a significant schema evolution that:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de6f1445d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| feature_definition: Mapped[dict[str, object]] = mapped_column( | ||
| JSONB, | ||
| nullable=True, | ||
| default=None, | ||
| comment="Language-specific query tweaks for adapter construction", | ||
| ) | ||
| description: Mapped[Optional[str]] = mapped_column(default=None) | ||
| startpoint: Mapped[bool] = mapped_column( | ||
| default=False, | ||
| comment="Indicates whether this feature represents a starting point or entry point in the application" | ||
| nullable=False, | ||
| default=dict, | ||
| server_default=text("'{}'::jsonb"), |
There was a problem hiding this comment.
FrameworkFeature no longer maps description, target_level, concept, locator_strategy, construct_query, or startpoint as ORM attributes, so the existing constructor calls that still pass these kwargs now fail at runtime (e.g. framework_loader.parse_json_data in code-confluence-flow-bridge creates FrameworkFeature(..., target_level=..., concept=..., ...)). With this commit, SQLAlchemy’s declarative constructor will raise TypeError for those unexpected keyword arguments, which prevents framework definition ingestion from loading at startup.
Useful? React with 👍 / 👎.
| def startpoint(self) -> bool: | ||
| """Return startpoint from feature_definition.""" | ||
| value = self.feature_definition["startpoint"] | ||
| if isinstance(value, bool): |
There was a problem hiding this comment.
After this change, startpoint is a plain Python @property, so class-level uses like select(FrameworkFeature.startpoint) no longer produce a SQL expression and instead pass a property object; query construction in code_confluence_framework_repository.py currently does exactly that in two read paths. This causes runtime query failures for framework-feature lookups unless all call sites are rewritten to use startpoint_sql_expression() (or the model exposes a hybrid/expression-backed attribute).
Useful? React with 👍 / 👎.
fix(optimise-framework-schema): move framework feature fields to JSON…
1 participant
…B payload