Skip to content

Conversation

OskarStark
Copy link
Contributor

@OskarStark OskarStark commented Sep 15, 2025

Q A
Bug fix? no
New feature? yes
Docs? no
Issues --
License MIT

Replace scalarNode() with more specific node types for better configuration type validation:

  • Use stringNode() for string values (API keys, URLs, service names, etc.)
  • Use integerNode() for integer values (dimensions, etc.)
  • Keep scalarNode() only for mixed-type fields where appropriate

This provides better type validation and makes the configuration more self-documenting.

Changes Made

  • String fields: API keys, URLs, endpoints, service names, class names, database/collection names, etc. now use stringNode()
  • Integer fields: dimensions and top_k fields now use integerNode()
  • Mixed fields: Strategy, metric, and similar fields remain as scalarNode()

Needs

@OskarStark OskarStark self-assigned this Sep 15, 2025
@OskarStark OskarStark added the AI Bundle Issues & PRs about the AI integration bundle label Sep 15, 2025
@carsonbot carsonbot changed the title Use stringNode() for string configuration values [AI Bundle] Use stringNode() for string configuration values Sep 15, 2025
@OskarStark OskarStark changed the title [AI Bundle] Use stringNode() for string configuration values [AI Bundle] Use stringNode() and integerNode() for better type validation Sep 15, 2025
@OskarStark OskarStark changed the title [AI Bundle] Use stringNode() and integerNode() for better type validation [AI Bundle] Use stringNode() and integerNode() for string configuration values Sep 15, 2025
Copy link
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be wrong but I feel like a lot of remaining scalarNode could be stringNode

@OskarStark OskarStark force-pushed the string-node branch 2 times, most recently from 9cc1570 to 9c95b45 Compare September 17, 2025 12:33
OskarStark added a commit that referenced this pull request Sep 17, 2025
…tring configuration values (OskarStark)

This PR was squashed before being merged into the main branch.

Discussion
----------

[AI Bundle] Use `stringNode()` and `integerNode()` for string configuration values

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| Docs?         | no
| Issues        | --
| License       | MIT

Replace `scalarNode()` with more specific node types for better configuration type validation:

- Use `stringNode()` for string values (API keys, URLs, service names, etc.)
- Use `integerNode()` for integer values (dimensions, etc.)
- Keep `scalarNode()` only for mixed-type fields where appropriate

This provides better type validation and makes the configuration more self-documenting.

## Changes Made

- **String fields**: API keys, URLs, endpoints, service names, class names, database/collection names, etc. now use `stringNode()`
- **Integer fields**: `dimensions` and `top_k` fields now use `integerNode()`
- **Mixed fields**: Strategy, metric, and similar fields remain as `scalarNode()`

## Needs
- [ ] #277

Commits
-------

492f75f [AI Bundle] Use `stringNode()` and `integerNode()` for string configuration values
@OskarStark OskarStark closed this Sep 17, 2025
@OskarStark OskarStark deleted the string-node branch September 23, 2025 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AI Bundle Issues & PRs about the AI integration bundle Feature New feature Status: Reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants