Skip to content

Conversation

@ElyarSadig
Copy link
Collaborator

@ElyarSadig ElyarSadig commented Oct 17, 2025

Pull Request

Related issue

Fixes #722

What does this PR do?

  • Fix typo tolerance settings update.
  • Improve typo tolerance update integration test.

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Summary by CodeRabbit

  • Changes
    • Updated typo tolerance settings to always include all configuration fields in API responses, ensuring consistent data formatting regardless of whether values are set to defaults.

@coderabbitai
Copy link

coderabbitai bot commented Oct 17, 2025

Walkthrough

The pull request fixes JSON serialization of TypoTolerance settings by removing omitempty tags from struct fields, ensuring zero-value fields are always included during marshaling. A test case verifying zero MinWordSizeForTypos behavior is added.

Changes

Cohort / File(s) Summary
Type definitions
types.go
Removed omitempty from JSON tags in TypoTolerance struct fields (MinWordSizeForTypos, DisableOnWords, DisableOnAttributes, DisableOnNumbers) and MinWordSizeForTypos struct fields (OneTypo, TwoTypos) to ensure zero values are always serialized.
Test updates
integration/index_settings_test.go
Added cleanup registrations for both meili and customMeili client instances and introduced new test case TestIndexZeroMinWordSizeForTypos to verify zero min word size behavior for typos.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

The changes involve removing omitempty JSON tags consistently across struct fields and adding straightforward test cleanup/verification logic. The modifications follow a repetitive pattern and address a specific serialization issue without introducing complex logic.

Possibly related PRs

  • #651: Modifies TypoTolerance settings structure by adding DisableOnNumbers field alongside this PR's changes to JSON serialization behavior.

Suggested labels

enhancement

Suggested reviewers

  • ja7ad

Poem

🐰 Zero values now shine bright,
No more omitting fields in sight,
JSON marshals full and true,
TypoTolerance works for you!
Tests ensure no typos creep,
While cleanup keeps things neat and deep. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "update: fix update typo tolerance" is directly related to the main change in the pull request. The PR addresses the issue where UpdateTypoTolerance was not updating MeiliSearch settings due to a JSON marshaling problem, and the code changes fix this by removing omitempty tags from TypoTolerance struct fields. While the title has slight redundancy in phrasing ("update: fix update"), it is specific and clear enough for a developer scanning the history to understand that this PR resolves the typo tolerance update functionality.
Linked Issues Check ✅ Passed The code changes directly address the requirements specified in issue #722. The root cause of the UpdateTypoTolerance malfunction was that struct fields with omitempty JSON tags containing zero values were being omitted from JSON marshaling, resulting in minWordSizeForTypos: {}. By removing omitempty from the JSON tags in types.go for TypoTolerance and MinWordSizeForTypos fields, these fields will now always be serialized, ensuring the typo tolerance settings (including minWordSizeForTypos with OneTypo and TwoTypos values) are properly transmitted. The test improvements in integration_test.go validate this fix works correctly by adding a test case for zero min word size behavior.
Out of Scope Changes Check ✅ Passed All changes in the pull request are directly related to fixing issue #722. The modifications to types.go remove omitempty JSON tags from TypoTolerance and MinWordSizeForTypos fields, which directly addresses the JSON marshaling issue identified in the problem statement. The test improvements in integration/index_settings_test.go add cleanup calls and a new test case to validate the fix works correctly. No extraneous changes or refactoring unrelated to the typo tolerance update issue are present.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dbe0e53 and 6fef404.

📒 Files selected for processing (2)
  • integration/index_settings_test.go (2 hunks)
  • types.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
integration/index_settings_test.go (1)
types.go (3)
  • TypoTolerance (178-184)
  • MinWordSizeForTypos (187-190)
  • TaskInfo (340-346)
🔇 Additional comments (4)
integration/index_settings_test.go (2)

3525-3554: LGTM! New test case verifies zero-value fix.

The new test case TestIndexZeroMinWordSizeForTypos correctly verifies that zero values for OneTypo and TwoTypos are properly serialized and round-tripped to the server. This validates the fix for issue #722 where zero values were being omitted during JSON marshaling.


3357-3358: Cleanup pattern differs from file conventions; verify test isolation is maintained.

The review comment's observation is correct: cleanup has moved from inside test loops (lines 46, 92, 132) to the function level. This is a real structural difference.

In TestIndex_UpdateTypoTolerance:

  • Both meili and customMeili clients are created at function level and reused across multiple subtests
  • t.Cleanup() is called once at function level for each client, after ALL subtests complete
  • Multiple subtests use the same client (e.g., tests 1, 3, 4 all use meili)

In contrast, other tests in the file (e.g., TestIndex_UpdateSortableAttributes):

  • Create a fresh client per subtest via setup(t, ...)
  • Call t.Cleanup() inside each subtest, ensuring cleanup between iterations

Since each TestIndex_UpdateTypoTolerance subtest calls setUpIndexForFaceting() and multiple subtests share the same index, verify that:

  • Index state does not accumulate unexpectedly between subtests
  • Document additions from setUpIndexForFaceting() don't cause conflicts (e.g., duplicates)
  • Test assertions remain valid when subtests run sequentially on shared index state
types.go (2)

180-183: LGTM! Removal of omitempty fixes JSON serialization issue.

Removing omitempty from these TypoTolerance fields ensures that zero-value and empty fields are always included in the JSON output. This directly addresses issue #722 where minWordSizeForTypos was becoming an empty object {} during marshaling.

Without omitempty:

  • DisableOnWords: []string{}"disableOnWords": []
  • DisableOnNumbers: false"disableOnNumbers": false

This ensures the API receives properly structured data even when fields have zero values.


188-189: LGTM! Zero values will now serialize correctly.

Removing omitempty from MinWordSizeForTypos fields is essential for the fix. Previously, when OneTypo: 0 and TwoTypos: 0 were set, the JSON marshaler would omit these fields due to omitempty, resulting in an incomplete minWordSizeForTypos object that confused the server.

Now these fields will always be present:

  • OneTypo: 0"oneTypo": 0
  • TwoTypos: 0"twoTypos": 0

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ja7ad ja7ad added the bug Something isn't working label Oct 17, 2025
@codecov
Copy link

codecov bot commented Oct 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.98%. Comparing base (dbe0e53) to head (6fef404).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #723   +/-   ##
=======================================
  Coverage   85.98%   85.98%           
=======================================
  Files          22       22           
  Lines        3854     3854           
=======================================
  Hits         3314     3314           
  Misses        383      383           
  Partials      157      157           

☔ 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.

@ja7ad ja7ad added this pull request to the merge queue Oct 17, 2025
Merged via the queue into meilisearch:main with commit 7521f85 Oct 17, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UpdateTypoTolerance function not updating typo tolerance setting

2 participants