feat(deployment): validate nextclade preprocessing configFile schema

[theosanderson]

Adds a bit of the values.yaml JSON schema that checks that the config settings applied to the nextclade-preprocessing pipeline are valid. This is quite vibe-coded but the fact that it passes suggests it's mostly good. The alternative implementation would be to build this into the preprocessing pipeline itself. That would be more consistent with how custom user pipelines would need to work, but less consistent with our other validation. For now, merging this PR probably makes sense to me.

Claude's description

Add conditional JSON schema validation for the nextclade preprocessing pipeline configFile. Validation is only applied when the image contains "preprocessing-nextclade", allowing other pipelines to have flexible configuration.

For nextclade pipelines:

• Add additionalProperties: false to configFile to reject unexpected keys and catch typos
• Add additionalProperties: false to nextclade_sequence_and_datasets items
• Add items.type: string to accepted_sort_matches and genes arrays
• Add missing config properties: batch_size (with minimum: 1)
• Add EMBL file generation options: create_embl_file, scientific_name, molecule_type (enum), topology (enum), db_name, and embl object
• Require nextclade_sequence_and_datasets field

For other pipelines (e.g., preprocessing-dummy):

• No strict validation applied, allowing flexible configuration

🚀 Preview: Add preview label to enable

[claude]

Code Review for PR #5678

I've reviewed the changes to add conditional JSON schema validation for the nextclade preprocessing pipeline. Overall, this is a solid improvement that will help catch configuration errors early. Here are my findings:

✅ Strengths

1. Smart Conditional Validation: The use of allOf with if/then to conditionally apply strict validation only to nextclade pipelines is elegant and allows flexibility for other pipeline types.

2. Comprehensive Coverage: The schema now covers all config fields defined in preprocessing/nextclade/src/loculus_preprocessing/config.py, including the recently added EMBL generation features.

3. Strict Validation: Adding additionalProperties: false at multiple levels will effectively catch typos and invalid configuration keys.

🔍 Issues Found

1. Missing log_level enum values (Medium Priority)

The schema defines:

"log_level": {
  "enum": ["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"]
}

However, in config.py:56, the default is "DEBUG" (string), and the code at line 186-188 reads from environment variable PREPROCESSING_LOG_LEVEL. The schema correctly lists Python logging levels, but should verify this matches Python's logging module expectations. ✅ This appears correct.

2. Type Mismatch: embl field (Low Priority)

Location: values.schema.json:653-672

The schema defines embl.admin_level_properties as:

"admin_level_properties": {
  "type": "array",
  "items": {
    "type": "string"
  }
}

In config.py:24-26, the default is:

admin_level_properties: list[str] = dataclasses.field(
    default_factory=lambda: ["geoLocAdmin1", "geoLocAdmin2", "geoLocCity", "geoLocSite"]
)

This is correct, but the schema should add "description" for the array items similar to other array fields for consistency.

3. Missing Fields from config.py (High Priority)

The schema is missing several fields that exist in the Config dataclass:

• processing_spec (config.py:72) - This is a dict[str, dict[str, Any]] and may be important for validation
• Fields related to authentication (lines 62-66): backend_host, keycloak_host, keycloak_user, keycloak_password, keycloak_token_path, organism, pipeline_version

However, these might be intentionally omitted from the schema if they're set elsewhere (environment variables, CLI args). Could you clarify if these should be in configFile or are managed separately?

4. Inconsistent Enum String Values (High Priority)

Location: values.schema.json:607-610

The molecule_type enum uses:

"enum": ["genomic DNA", "genomic RNA", "viral cRNA"]

But in datatypes.py:218-221, the enum is:

class MoleculeType(StrEnum):
    GENOMIC_DNA = "genomic DNA"
    GENOMIC_RNA = "genomic RNA"
    VIRAL_CRNA = "viral cRNA"  # ⚠️ Note: "viral cRNA" not "VIRAL_CRNA"

✅ Schema correctly uses the string values, not the enum keys. Good!

📋 Recommendations

Must Fix:

1. Clarify scope of validation: Document which config fields are expected in configFile vs. set via environment variables or other means. The schema now requires nextclade_sequence_and_datasets but this field has a default value in the code.

Should Consider:

2. Add schema version: Consider adding a $schema or schemaVersion field to track schema evolution.

3. Test coverage: Add integration tests that verify:

   • Valid nextclade configs pass validation
   • Invalid configs (typos, wrong types) are properly rejected
   • Other pipeline types (preprocessing-dummy) aren't affected by strict validation

4. Documentation: Update the preprocessing pipeline documentation to reference this schema validation, especially for users creating custom pipelines.

5. Validation error messages: Test what error messages users see when validation fails - JSON schema errors can be cryptic.

🔐 Security

No security concerns identified. The schema validation will actually help prevent injection of unexpected configuration values.

⚡ Performance

Schema validation is negligible performance overhead during deployment/startup, which is appropriate.

✨ Style & Conventions

The PR follows the conventional commit format correctly: feat(deployment): validate nextclade preprocessing configFile schema. The detailed PR description is excellent and follows the project's guidelines for detailed summaries.

🧪 Testing

Missing: No test coverage mentioned for the schema validation itself. Consider adding tests in the deployment/kubernetes test suite or integration tests that verify:

• Helm chart validation with valid configs
• Helm chart rejection with invalid configs

---

Overall Assessment: This is a valuable addition that will catch configuration errors early. The conditional validation approach is well-designed. Main concerns are around ensuring all necessary fields are included and adding test coverage for the validation logic.

The "vibe-coded" nature mentioned in the PR description suggests this would benefit from systematic testing to ensure completeness.

🚀 Preview link looks good for manual validation!

[theosanderson]

This whole section is only needed so it shows up in the docs.. maybe this could be done another way to avoid code duplication but I can't right now. (Or we could just ignore the docs on this aspect)

[claude]

Code Review for PR #5678

I've reviewed this PR that adds conditional JSON schema validation for the nextclade preprocessing pipeline. Overall, this is a valuable improvement that will catch configuration errors early. Here are my findings:

✅ Strengths

1. Smart conditional validation: The use of allOf with if/then to conditionally apply strict validation only to nextclade pipelines is a clean approach that allows flexibility for custom pipelines like preprocessing-dummy.

2. Improved error detection: Adding additionalProperties: false at multiple levels will help catch typos and unexpected configuration keys, which is the main goal of this PR.

3. Comprehensive coverage: The schema now includes previously undocumented fields like batch_size, EMBL file generation options, and log_level.

4. Array item validation: Adding items.type: string to accepted_sort_matches and genes arrays provides proper type safety.

[claude]

🐛 Issues Found

1. Critical: Field name inconsistency (config.py:144)

There's a mismatch between the schema and the Python code:

• Schema uses: "embl" (line 652 in new schema)
• Python code expects: "embl_info" (preprocessing/nextclade/src/loculus_preprocessing/config.py:144)

The Python code has: if key == "embl_info" and isinstance(value, dict):

This means the embl configuration will never be properly loaded from the config file. The schema field name should be changed to "embl_info" to match the Python code, or the Python code should be updated to use "embl".

Recommendation: Change the schema to use "embl_info" since that's what the Python code expects, or coordinate a change in both places if you prefer "embl".

[claude]

2. Missing validation for empty properties

In the then clause (lines 605-638), the properties are defined with empty objects {}. While this allows the properties to exist, it loses all the detailed validation from the nextcladeConfigFile definition. This means:

• Type checking is lost
• Enum validation is lost
• Descriptions are lost

For example, alignment_requirement should still validate as an enum even when inside the conditional.

Recommendation: Consider using $ref to reference the definitions from nextcladeConfigFile, or duplicate the validation rules.

[claude]

3. Incomplete EMBL validation (line 652)

The admin_level_properties field in the schema doesn't specify items.type: "string", even though it's an array of strings according to the Python code (config.py:24-26).

Fix needed: Add proper array item validation for admin_level_properties.

⚠️ Minor Concerns

1. Schema duplication: The field definitions now exist in two places - the nextcladeConfigFile definition and inline within the preprocessing pipeline schema. This creates maintenance burden.

2. Missing minimum constraint propagation: The batch_size field has minimum: 1 in the nextcladeConfigFile definition but this constraint isn't present in the conditional validation (line 626).

3. Documentation consistency: The description on line 587 references nextcladeConfigFile but the link format may not render correctly.

[claude]

🧪 Testing Recommendations

1. Test with a config that has embl vs embl_info to verify which one actually works
2. Test with a typo in a field name to ensure additionalProperties: false catches it
3. Test with preprocessing-dummy to ensure it's not affected by the strict validation
4. Test with invalid enum values (e.g., molecule_type: "invalid") to ensure validation works

📝 Summary

This PR provides valuable validation, but the embl vs embl_info naming inconsistency is a blocking issue that will prevent EMBL configurations from working correctly. Please fix this before merging. The empty property definitions in the conditional validation also reduce the effectiveness of the schema validation.

Verdict: Request changes - fix the embl/embl_info inconsistency and consider improving the conditional validation to preserve type/enum checks.