-
Notifications
You must be signed in to change notification settings - Fork 1
Name length limit to facets.yaml json schema #76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds an optional integer property name_length_limit (minimum 1) to yaml_schema.properties in ftf_cli/schema.py; not added to the required list. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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. ✨ Finishing Touches
🧪 Generate unit tests
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ftf_cli/schema.py (1)
107-110: Clarify intent with a description field (nit).Adding a short description will help downstream tooling and future maintainers understand how this knob is consumed.
Apply this diff:
"name_length_limit": { "type": "integer", "minimum": 1, + "description": "Maximum allowed length for names; consumed by the CLI for validation (optional).", }
📜 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 selected for processing (1)
ftf_cli/schema.py(1 hunks)
🔇 Additional comments (1)
ftf_cli/schema.py (1)
107-110: LGTM: Schema addition is valid and optional as intended.The new top-level property is correctly typed as an integer with a lower bound, and not added to required — matches the PR objective.
| "name_length_limit": { | ||
| "type": "integer", | ||
| "minimum": 1, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Confirm enforcement path; schema alone won’t constrain other names.
As added, this is metadata that validators will accept, but it won’t automatically enforce name lengths elsewhere in the schema (JSON Schema can’t dynamically reference this value for other fields without extensions). Please verify that the CLI/runtime reads this property and applies the limit where intended (e.g., for inputs/outputs/spec keys).
[offer_assistance]
Run this script to locate usages and tests:
If there’s no consumer yet, I can help add a small validator hook and tests—want me to draft that?
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching code for name_length_limit usage..."
rg -n -C3 '\bname_length_limit\b' || echo "No code references found."
echo
echo "Searching for name length validation logic that might need to read this property..."
rg -n -C3 -i '\b(name.*length|length.*name|length_limit|max[_-]?length)\b' || echo "No obvious validators found."
echo
echo "Looking for tests touching name_length_limit..."
rg -n -C2 -i 'name_length_limit' -g '!*node_modules/*' -g '!*dist/*' || echo "No tests found referencing name_length_limit."Length of output: 1236
Implement enforcement for name_length_limit
The name_length_limit setting is declared in ftf_cli/schema.py but there’s no runtime logic or tests consuming it. To ensure name-length constraints actually take effect, you should:
- Add a validator in your CLI/runtime (e.g. in
ftf_cli/validate.py) that readsschema["name_length_limit"]and enforces it on all relevant names (inputs, outputs, spec keys). - Write unit tests (e.g.
tests/test_schema_validators.py) to verify names below, at, and above the limit are accepted or rejected as expected.
Let me know if you’d like me to draft the validator hook and accompanying tests.
Summary by CodeRabbit