Fix bugs in JSON and SQA storage modules by saitcakmak · Pull Request #4893 · facebook/Ax

saitcakmak · 2026-02-11T14:20:06Z

Summary:
Fix multiple bugs in ax/storage:

Missing raise before SQADecodeError silently ignores JSON mismatches during copy_db_ids (sqa_store/utils.py:114)
int(None) crash in SQA encoder: metric_registry.get() can return None, but the result was immediately passed to int() before the None check could run (sqa_store/encoder.py:393)
AttributeError on None kwargs in JSON decoder: kwargs.pop() was called before the if kwargs is not None guard (json_store/decoder.py:1092)
decode_args_list length mismatch in SQA save: used len(trials) instead of len(trials_to_reduce_state) and [...] * len(trials) instead of a single-element list (sqa_store/save.py:245,254)
SQLAlchemy filter no-op: SQAExperiment.id is not None uses Python identity check instead of SQLAlchemy's .isnot(None) (sqa_store/load.py:760)
Inconsistent threshold in error message: says "> 10" but the actual check is > 15 (sqa_store/utils.py:77)
Garbled error message in with_db_settings_base.py: sentence fragments in wrong order (sqa_store/with_db_settings_base.py:106)
Doubled path ax/ax/storage in error message (sqa_store/encoder.py:131)
Backslash line-continuation inside string literal causes 16 extra spaces in error message (sqa_store/encoder.py:654)
Missing space between concatenated strings (sqa_store/db.py)
Typo "SCalarized" → "Scalarized" (sqa_store/encoder.py)
Missing space in SKIP_ATTRS_ERROR_SUFFIX (sqa_store/utils.py)

Differential Revision: D92879499

meta-codesync · 2026-02-11T14:20:47Z

@saitcakmak has exported this pull request. If you are a Meta employee, you can view the originating Diff in D92879499.

codecov-commenter · 2026-02-11T14:52:40Z

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.83%. Comparing base (bb4a3f6) to head (74922e7).

Files with missing lines	Patch %	Lines
ax/storage/sqa_store/utils.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4893   +/-   ##
=======================================
  Coverage   96.83%   96.83%           
=======================================
  Files         594      594           
  Lines       62873    62857   -16     
=======================================
- Hits        60880    60865   -15     
+ Misses       1993     1992    -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Summary: Fix multiple bugs in ax/storage: - Missing `raise` before `SQADecodeError` silently ignores JSON mismatches during `copy_db_ids` (sqa_store/utils.py:114) - `int(None)` crash in SQA encoder: `metric_registry.get()` can return None, but the result was immediately passed to `int()` before the None check could run (sqa_store/encoder.py:393) - `AttributeError` on None kwargs in JSON decoder: `kwargs.pop()` was called before the `if kwargs is not None` guard (json_store/decoder.py:1092) - `decode_args_list` length mismatch in SQA save: used `len(trials)` instead of `len(trials_to_reduce_state)` and `[...] * len(trials)` instead of a single-element list (sqa_store/save.py:245,254) - SQLAlchemy filter no-op: `SQAExperiment.id is not None` uses Python identity check instead of SQLAlchemy's `.isnot(None)` (sqa_store/load.py:760) - Inconsistent threshold in error message: says "> 10" but the actual check is `> 15` (sqa_store/utils.py:77) - Garbled error message in with_db_settings_base.py: sentence fragments in wrong order (sqa_store/with_db_settings_base.py:106) - Doubled path `ax/ax/storage` in error message (sqa_store/encoder.py:131) - Backslash line-continuation inside string literal causes 16 extra spaces in error message (sqa_store/encoder.py:654) - Missing space between concatenated strings (sqa_store/db.py) - Typo "SCalarized" → "Scalarized" (sqa_store/encoder.py) - Missing space in SKIP_ATTRS_ERROR_SUFFIX (sqa_store/utils.py) Differential Revision: D92879499

Summary: Fix multiple bugs in ax/storage: - `int(None)` crash in SQA encoder: `metric_registry.get()` can return None, but the result was immediately passed to `int()` before the None check could run (sqa_store/encoder.py:393) - `AttributeError` on None kwargs in JSON decoder: `kwargs.pop()` was called before the `if kwargs is not None` guard (json_store/decoder.py:1092) - `decode_args_list` length mismatch in SQA save: used `len(trials)` instead of `len(trials_to_reduce_state)` and `[...] * len(trials)` instead of a single-element list (sqa_store/save.py:245,254) - SQLAlchemy filter no-op: `SQAExperiment.id is not None` uses Python identity check instead of SQLAlchemy's `.isnot(None)` (sqa_store/load.py:760) - Inconsistent threshold in error message: says "> 10" but the actual check is `> 15` (sqa_store/utils.py:77) - Garbled error message in with_db_settings_base.py: sentence fragments in wrong order (sqa_store/with_db_settings_base.py:106) - Doubled path `ax/ax/storage` in error message (sqa_store/encoder.py:131) - Backslash line-continuation inside string literal causes 16 extra spaces in error message (sqa_store/encoder.py:654) - Missing space between concatenated strings (sqa_store/db.py) - Typo "SCalarized" → "Scalarized" (sqa_store/encoder.py) - Missing space in SKIP_ATTRS_ERROR_SUFFIX (sqa_store/utils.py) Reviewed By: bernardbeckerman Differential Revision: D92879499

Summary: Pull Request resolved: facebook#4893 Fix multiple bugs in ax/storage: - `int(None)` crash in SQA encoder: `metric_registry.get()` can return None, but the result was immediately passed to `int()` before the None check could run (sqa_store/encoder.py:393) - `AttributeError` on None kwargs in JSON decoder: `kwargs.pop()` was called before the `if kwargs is not None` guard (json_store/decoder.py:1092) - `decode_args_list` length mismatch in SQA save: used `len(trials)` instead of `len(trials_to_reduce_state)` and `[...] * len(trials)` instead of a single-element list (sqa_store/save.py:245,254) - SQLAlchemy filter no-op: `SQAExperiment.id is not None` uses Python identity check instead of SQLAlchemy's `.isnot(None)` (sqa_store/load.py:760) - Inconsistent threshold in error message: says "> 10" but the actual check is `> 15` (sqa_store/utils.py:77) - Garbled error message in with_db_settings_base.py: sentence fragments in wrong order (sqa_store/with_db_settings_base.py:106) - Doubled path `ax/ax/storage` in error message (sqa_store/encoder.py:131) - Backslash line-continuation inside string literal causes 16 extra spaces in error message (sqa_store/encoder.py:654) - Missing space between concatenated strings (sqa_store/db.py) - Typo "SCalarized" → "Scalarized" (sqa_store/encoder.py) - Missing space in SKIP_ATTRS_ERROR_SUFFIX (sqa_store/utils.py) Reviewed By: bernardbeckerman Differential Revision: D92879499

Summary: The `JSON_ATTRS` list and its associated code path in `copy_db_ids` were introduced in D35400247 to prevent `copy_db_ids` from recursing into `baseline_workflow_inputs`, a plain JSON dict with no `_db_id` fields or nested `Base` objects. The JSON_ATTRS path was meant to check source/target equality without recursing, but it had a bug from day one: the `SQADecodeError` on mismatch was constructed but never `raise`d, so the equality check was silently discarded. Since its only entry (`baseline_workflow_inputs`) is a plain JSON blob with no `_db_id` fields, there is nothing for `copy_db_ids` to do with it. Moving it to `COPY_DB_IDS_ATTRS_TO_SKIP` (which skips the attribute entirely) preserves the actual runtime behavior while removing the dead code path. This also removes the `DummyClassWithBaseline` test class and `test_json_copy_db_ids` test, which existed solely to exercise the now-removed JSON_ATTRS path. Differential Revision: D93664682

Summary: Fix multiple bugs in ax/storage: - `int(None)` crash in SQA encoder: `metric_registry.get()` can return None, but the result was immediately passed to `int()` before the None check could run (sqa_store/encoder.py:393) - `AttributeError` on None kwargs in JSON decoder: `kwargs.pop()` was called before the `if kwargs is not None` guard (json_store/decoder.py:1092) - `decode_args_list` length mismatch in SQA save: used `len(trials)` instead of `len(trials_to_reduce_state)` and `[...] * len(trials)` instead of a single-element list (sqa_store/save.py:245,254) - SQLAlchemy filter no-op: `SQAExperiment.id is not None` uses Python identity check instead of SQLAlchemy's `.isnot(None)` (sqa_store/load.py:760) - Inconsistent threshold in error message: says "> 10" but the actual check is `> 15` (sqa_store/utils.py:77) - Garbled error message in with_db_settings_base.py: sentence fragments in wrong order (sqa_store/with_db_settings_base.py:106) - Doubled path `ax/ax/storage` in error message (sqa_store/encoder.py:131) - Backslash line-continuation inside string literal causes 16 extra spaces in error message (sqa_store/encoder.py:654) - Missing space between concatenated strings (sqa_store/db.py) - Typo "SCalarized" → "Scalarized" (sqa_store/encoder.py) - Missing space in SKIP_ATTRS_ERROR_SUFFIX (sqa_store/utils.py) Reviewed By: bernardbeckerman Differential Revision: D92879499

Summary: Pull Request resolved: facebook#4893 Fix multiple bugs in ax/storage: - `int(None)` crash in SQA encoder: `metric_registry.get()` can return None, but the result was immediately passed to `int()` before the None check could run (sqa_store/encoder.py:393) - `AttributeError` on None kwargs in JSON decoder: `kwargs.pop()` was called before the `if kwargs is not None` guard (json_store/decoder.py:1092) - `decode_args_list` length mismatch in SQA save: used `len(trials)` instead of `len(trials_to_reduce_state)` and `[...] * len(trials)` instead of a single-element list (sqa_store/save.py:245,254) - SQLAlchemy filter no-op: `SQAExperiment.id is not None` uses Python identity check instead of SQLAlchemy's `.isnot(None)` (sqa_store/load.py:760) - Inconsistent threshold in error message: says "> 10" but the actual check is `> 15` (sqa_store/utils.py:77) - Garbled error message in with_db_settings_base.py: sentence fragments in wrong order (sqa_store/with_db_settings_base.py:106) - Doubled path `ax/ax/storage` in error message (sqa_store/encoder.py:131) - Backslash line-continuation inside string literal causes 16 extra spaces in error message (sqa_store/encoder.py:654) - Missing space between concatenated strings (sqa_store/db.py) - Typo "SCalarized" → "Scalarized" (sqa_store/encoder.py) - Missing space in SKIP_ATTRS_ERROR_SUFFIX (sqa_store/utils.py) Reviewed By: bernardbeckerman Differential Revision: D92879499

meta-codesync · 2026-02-25T17:24:33Z

This pull request has been merged in 9e1bb8f.

meta-cla bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Feb 11, 2026

meta-codesync bot added fb-exported meta-exported labels Feb 11, 2026

saitcakmak force-pushed the export-D92879499 branch 2 times, most recently from c3f4e9c to a09d6fe Compare February 18, 2026 14:41

saitcakmak force-pushed the export-D92879499 branch from a09d6fe to 5f77bc4 Compare February 25, 2026 01:42

saitcakmak force-pushed the export-D92879499 branch from 5f77bc4 to 1e1b2c7 Compare February 25, 2026 04:48

saitcakmak force-pushed the export-D92879499 branch from 1e1b2c7 to 3b50c73 Compare February 25, 2026 04:51

saitcakmak force-pushed the export-D92879499 branch from 3b50c73 to dfa8aad Compare February 25, 2026 04:51

saitcakmak force-pushed the export-D92879499 branch from dfa8aad to df24776 Compare February 25, 2026 15:05

saitcakmak force-pushed the export-D92879499 branch from df24776 to 74922e7 Compare February 25, 2026 15:10

meta-codesync bot closed this in 9e1bb8f Feb 25, 2026

facebook-github-bot added the Merged label Feb 25, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix bugs in JSON and SQA storage modules#4893

Fix bugs in JSON and SQA storage modules#4893
saitcakmak wants to merge 2 commits intofacebook:mainfrom
saitcakmak:export-D92879499

saitcakmak commented Feb 11, 2026

Uh oh!

meta-codesync bot commented Feb 11, 2026

Uh oh!

codecov-commenter commented Feb 11, 2026 •

edited

Loading

Uh oh!

meta-codesync bot commented Feb 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

saitcakmak commented Feb 11, 2026

Uh oh!

meta-codesync bot commented Feb 11, 2026

Uh oh!

codecov-commenter commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

meta-codesync bot commented Feb 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Feb 11, 2026 •

edited

Loading