fix: [AI-9453] parse dbt-core 1.11+ manifests and stop downgrading dbt deps

[saravmajestic]

Jira: AI-9453

Problem

dbt Power User's Project Governance fails for every project on dbt-core 1.11+, with any datapilot version up to and including 0.3.7.

• dbt-core 1.11+ ships the built-in macro materialization_function_default with supported_languages: [sql, python, javascript]. Manifest v12 SupportedLanguage only allowed python / sql, so parsing failed.
• The disabled fallback dropped the key, but v11/v12 declare disabled required (nullable). The retry therefore always failed with disabled: Field required, which hid the real error.
• click~=8.1.7 and python-dotenv~=1.0.0 downgrade packages dbt-core 1.11+ requires (click>=8.3, python-dotenv>=1.2), so installing datapilot breaks the user's dbt env.

Fix

• Add javascript to v12 SupportedLanguage.
• Point AltimateSupportedLanguage (schemas/manifest.py) at the v12 enum instead of v11. Otherwise ManifestV12Wrapper._get_macro still raised ValueError for any project-owned macro supporting JavaScript, e.g. a custom UDF materialization. v12's enum is a superset of v11's.
• The v10/v11 wrappers now convert macro supported_languages by value, like v12. Passing vendored enum members to the v12-typed field raised ValidationError (from review).
• The fallback sets disabled to None instead of removing it (_null_unused_fields). If the retry also fails, the original error is raised, and the retry is logged at debug level.
• Relax the pins to click>=8.1.7,<9.0 and python-dotenv>=1.0.0,<2.0.
• Add tests/test_vendor/test_manifest_v12.py and tests/core/platform/dbt/test_manifest_wrapper_macros.py (v10/v11).

Version bump to 0.3.8 is left to the usual bump PR. Companion PRs pin it: AltimateAI/altimate-backend#7000 (REQUIRED_VERSION) and AltimateAI/vscode-dbt-power-user#2085 (version check). Publish 0.3.8 to PyPI before the backend PR deploys.

Verification

Tests (python:3.10, latest click 8.5.0 / python-dotenv 1.2.4)

# origin/main + new tests
FAILED tests/test_vendor/test_manifest_v12.py::test_parses_javascript_supported_language
FAILED tests/test_vendor/test_manifest_v12.py::test_unparseable_disabled_falls_back_to_none
2 failed, 1 passed

# this branch, full suite
163 passed

# wrapper test with the alias reverted to v11
E   ValueError: 'javascript' is not a valid SupportedLanguage
FAILED tests/test_vendor/test_manifest_v12.py::test_wraps_project_macro_supporting_javascript

pre-commit (ruff, black, whitespace, EOF, debug-statements): all passed.

Manifest from dbt-core 1.12.5 (jaffle_shop)

0.3.7: Input should be 'python' or 'sql' [input_value='javascript']  +  disabled: Field required
0.3.8 (this branch): datapilot dbt project-health -> model insights reported (e.g. model_invalid_name)

After installing into a dbt-core 1.12.5 venv: pip check → No broken requirements found, and dbt parse OK.

End-to-end in dbt Power User (code-server, dbt-core 1.12.5, local 0.3.8 wheel)

Before (0.3.7 installed → forced to 0.2.0)	After (upgraded to 0.3.8)

Brand-new user (fresh install)	Re-scan with 0.3.8 installed (no prompt)

Re-verified after review fixes (damaged env + project JS materialization)

A dbt-core 1.12.5 venv already damaged by 0.3.7 (click 8.1.8, python-dotenv 1.0.1), with a project macro {% materialization ai9453_js_udf, default, supported_languages=['sql', 'javascript'] %}. Upgrading to 0.3.8 from the extension completed governance, including the JS macro file, with no ValueError.

🤖 Generated with Claude Code

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (5 files)

• src/datapilot/core/platforms/dbt/wrappers/manifest/v10/wrapper.py
• src/datapilot/core/platforms/dbt/wrappers/manifest/v11/wrapper.py
• src/vendor/dbt_artifacts_parser/parser.py
• tests/core/platform/dbt/test_manifest_wrapper_macros.py
• tests/test_vendor/test_manifest_v12.py

Previous Review Summaries (2 snapshots, latest commit 40f62c3)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 40f62c3)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (5 files)

• setup.py
• src/datapilot/core/platforms/dbt/schemas/manifest.py
• src/vendor/dbt_artifacts_parser/parser.py
• src/vendor/dbt_artifacts_parser/parsers/manifest/manifest_v12.py
• tests/test_vendor/test_manifest_v12.py

Previous review (commit 8b20353)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (4 files)

• setup.py
• src/vendor/dbt_artifacts_parser/parser.py
• src/vendor/dbt_artifacts_parser/parsers/manifest/manifest_v12.py
• tests/test_vendor/test_manifest_v12.py

---

_{Reviewed by gpt-6-sol · Input: 16 · Output: 2.9K · Cached: 293.3K}

[sahrizvi]

Review summary

Verdict: changes requested. One blocking regression (see the inline comment on schemas/manifest.py). Everything else below is non-blocking.

The dbt 1.11+ fix itself works. The full suite (161 tests) passes on this branch with click 8.5.0, python-dotenv 1.2.4 and pydantic 2.13.5.

Minor

1. Test coverage gaps (tests/test_vendor/test_manifest_v12.py)

• Nothing tests the v10/v11 get_macros() paths. That's how the inline regression got through.
• The 1.11 case clones a macro inside an older v12 fixture. A trimmed manifest from a real dbt-core 1.11+/1.12 project, with top-level functions and depends_on.functions, would cover that release's artifact shape. Optional.
• test_unrelated_errors_still_raise can't detect error masking. Both parse attempts fail with the same supported_languages error, and pydantic reports every error, so the test passes whichever exception _try_parse_manifest raises.

2. Fallback error handling is fragile and silent (src/vendor/dbt_artifacts_parser/parser.py:98-106; this predates the PR, so treat it as a follow-up)
Setting disabled to None fixes the masking failure described in this PR. Two older problems remain. When the retry fails, the retry's error is the one raised; the original survives only as __context__, and except Exception: raise does nothing. When the retry succeeds, an unparseable disabled section is discarded with no log line. Suggested:

except Exception as original:
    logger.debug("Manifest parse failed; retrying with %s nulled", _UNUSED_STRICT_FIELDS, exc_info=True)
    try:
        return model_class(**_strip_unused_fields(manifest))
    except Exception:
        raise original

Nits / optional

• setup.py: the new click>=8.1.7,<9.0 and python-dotenv>=1.0.0,<2.0 ranges are correct. A CI job against the lower bounds would be a nice-to-have. Don't tighten the upper bound, because dbt-core 1.11+ needs click>=8.3.
• schemas/manifest.py:425: Optional[Optional[List[AltimateSupportedLanguage]]] is redundant. This predates the PR.
• _strip_unused_fields: the function now nulls fields instead of stripping them, so _null_unused_fields would describe it better.

What's good

• The root-cause write-up is accurate, and the docstring is right: v11/v12 declare disabled as required but nullable, while v1–v10 default it to None. Setting it to None is safe for every version.
• The v12 enum is a strict superset of v11's, and the v12 wrapper already converts by value.
• The change is small and focused, the tests use GIVEN/WHEN/THEN and fail on main, and relaxing the pins fixes the real dbt env breakage.

[sahrizvi]

MAJOR: regression for dbt 1.7 (manifest v11) projects with custom materializations

Pointing AltimateSupportedLanguage at the v12 enum breaks the v11 wrapper. ManifestV11Wrapper._get_macro (wrappers/manifest/v11/wrapper.py:184) passes macro.supported_languages (v11 SupportedLanguage members) straight into AltimateManifestMacroNode.supported_languages, whose type is now List[v12.SupportedLanguage]. Pydantic v2 rejects a member of a different Enum class even when the value matches.

Repro: load tests/data/manifest_v11.json, make a macro that has supported_languages project-owned, then call DBTFactory.get_manifest_wrapper(parse_manifest(m)).get_macros().

• main: OK, [SupportedLanguage.sql]
• this branch: ValidationError: Input should be 'python', 'sql' or 'javascript' [type=enum, input_value=<SupportedLanguage.sql: 'sql'>, input_type=SupportedLanguage]

DBTInsightGenerator.__init__ (executor.py:63) always calls get_macros(). Project health / governance therefore crashes for any v11 project with a project-owned macro that declares supported_languages, which in practice means any custom materialization. 0.3.7 doesn't have this problem. The failure only needs "sql"; JavaScript plays no part. The v10 wrapper (v10/wrapper.py:184) uses the same pattern and already fails on main, and the same fix covers it.

Suggested fix: in the v10 and v11 wrappers, convert by value as the v12 wrapper already does:

supported_languages=[AltimateSupportedLanguage(lang.value) for lang in macro.supported_languages]
if macro.supported_languages
else None,

Add v10 and v11 counterparts of test_wraps_project_macro_supporting_javascript that use ["sql"]. Optionally, make AltimateSupportedLanguage a standalone enum here (python, sql, javascript) so a future vendor bump can't silently change it again.

[saravmajestic]

Good catch, thanks. I reproduced it on the branch with your steps; both v10 and v11 failed with the same ValidationError (input_value=<SupportedLanguage.sql: 'sql'>).

Fixed in e7d2163: the v10 and v11 wrappers now convert by value (AltimateSupportedLanguage(lang.value)), the same way the v12 wrapper does. Added tests/core/platform/dbt/test_manifest_wrapper_macros.py, parametrized over the v10 and v11 fixtures with a project-owned macro using ["sql"]. Both cases fail on 40f62c3 and pass now.

I kept AltimateSupportedLanguage as an alias of the v12 enum for now. Making it a standalone enum is a reasonable hardening step, and I'm happy to do it here if you'd prefer.

[sahrizvi]

Verified the fix in e7d2163. Both wrappers now convert by value, and the new parametrized test fails for v10 and v11 when the old wrappers are restored. Resolved. Keeping the alias is fine; the standalone enum can be a follow-up.

[saravmajestic]

Thanks for the thorough review. All changes are in e7d2163:

• Blocking (v10/v11 get_macros() regression): fixed. See the inline reply.
• Test gaps: added v10/v11 wrapper tests. I replaced test_unrelated_errors_still_raise with test_failed_fallback_raises_the_original_error, where the first attempt and the retry raise different errors and the test asserts the original is raised. It fails on 40f62c3 and passes now.
• Fallback handling: took your suggestion. The failed first attempt is logged at debug level, and if the retry also fails, the original error is raised.
• Rename: _strip_unused_fields is now _null_unused_fields.
• Not in this PR: the CI job for the lower bounds, the trimmed real dbt 1.12 fixture, and the redundant Optional[Optional[...]] (which predates this PR). Happy to do them as follow-ups.

Full suite: 163 passed. pre-commit (ruff, black, whitespace, EOF, debug-statements) all passed.

[sahrizvi]

Re-review (e7d2163)

No blocking issues left. This looks ready to merge. Every finding from the previous round is resolved:

• v10/v11 supported_languages ValidationError: fixed. Both wrappers convert by value, the same way v12 does. Nothing else in src/datapilot consumes SupportedLanguage/supported_languages, and v10, v11 and v12 are the only manifest wrappers.
• Fallback error handling: fixed. The first failure is logged at debug level, and if the retry also fails the original error is raised.
• Rename to _null_unused_fields: done, with no stale callers.

Verified: the full suite passes (163 tests). I mutation-checked both new regression tests:

• Restoring the 40f62c3 v10/v11 wrappers makes test_wraps_project_macro_with_supported_languages fail for both fixtures.
• Changing raise original back to raise makes test_failed_fallback_raises_the_original_error fail.

Both tests prove what they claim.

The two inline comments are optional nits. Follow-ups are fine for the items you deferred (standalone enum, CI job for the lower bounds, real dbt 1.12 fixture, Optional[Optional[...]]).

[sahrizvi]

Nit (optional): raising original inside the nested except attaches the retry error as __context__. The traceback prints the retry failure first, then "During handling of the above exception, another exception occurred", and only then the original. It's harmless, and the retry error can be useful context. If you'd prefer a cleaner traceback:

            raise original from None

[sahrizvi]

Nit (optional): the stub Model is the right way to prove the "original error wins" contract. Replacing test_unrelated_errors_still_raise did drop the end-to-end check that a real non-disabled validation error (the cobol case) still comes out of parse_manifest, though. Keeping both would cost only a few lines.

[sahrizvi]

Approving at e7d2163. All findings from the earlier review are resolved, and the two inline nits are optional.