fix(exists): return boolean values in JSON output instead of strings #108

Copilot · 2025-12-30T18:39:40Z

📥 Pull Request

✨ Description of new changes

The fab exists command was returning "true"/"false" strings in JSON output instead of native JSON booleans. This caused logic errors where "false" evaluates as truthy in conditionals.

Before:

{
  "result": { "message": "true" }  // String, not boolean
}

After:

{
  "result": { "message": true }  // Boolean
}

Changes

Type system: Updated OutputResult and FabricCLIOutput to accept message: str | bool
Output formatting: Added _format_message_for_text() helper to convert booleans to strings for text output, preserving backward compatibility
Exists command: Updated to use inline boolean values (True/False) instead of constants
Test fixture: Changed setup_default_format fixture to class scope to ensure format config is available for class-scoped fixtures like workspace
Code cleanup: Removed unnecessary comments, unused imports, and INFO_EXISTS_TRUE/INFO_EXISTS_FALSE constants based on code review feedback

Testing

Text output remains unchanged (displays "true"/"false" strings). All existing tests pass:

✅ 9 exists command tests pass with text output
✅ 36 fab_ui tests pass including boolean serialization tests for JSON output
✅ Test helper functions remain clean without config side effects
✅ Tests can override format configuration as needed

Original prompt

This section details on the original issue you should resolve

<issue_title>[BUG] fab exists returns strings instead of booleans in --output_format json</issue_title>
<issue_description>### Bug Description

When output_format=json is configured (either via config file or --output_format json argument) the exists command returns boolean values as strings "true" or "false" instead of native JSON booleans. This forces the user to parse strings instead of using boolean values directly. Furthermore this can cause logic errors as "false" is truthy in conditional checks.
fab exists ws1.Workspace/lh1.Lakehouse --output_format json
{
    "timestamp": "2025-12-29T16:42:07.387900Z",
    "status": "Success",
    "command": "exists",
    "result": {
        "message": "true"
    }
}
fabric-cli version

1.3.1.

Python version

3.12.3

Operating System

Windows

CLI Mode

Command line mode

Authentication Method

Service principal (secret)

Steps to Reproduce
fab exists <workspace>/<item> --output_format json
{
    "timestamp": "2025-12-29T16:42:07.387900Z",
    "status": "Success",
    "command": "exists",
    "result": {
        "message": "true"
    }
}
Expected Behavior
fab exists <workspace>/<item> --output_format json
{
    "timestamp": "2025-12-29T16:42:07.387900Z",
    "status": "Success",
    "command": "exists",
    "result": {
        "message": true
    }
}
Actual Behavior
fab exists <workspace>/<item> --output_format json
{
    "timestamp": "2025-12-29T16:42:07.387900Z",
    "status": "Success",
    "command": "exists",
    "result": {
        "message": "true"
    }
}
Additional Context

No response

Possible Solution

No response</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes [BUG] fab exists returns strings instead of booleans in --output_format json #107

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

- Changed INFO_EXISTS_TRUE and INFO_EXISTS_FALSE constants from strings to booleans - Updated OutputResult and FabricCLIOutput to accept bool | str message type - Added boolean-to-string conversion for text output format - Added default fallback to "text" format when format_type is None - Updated existing tests to work with boolean constants - Added new unit tests for boolean message JSON/text output Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com>

Extract inline boolean-to-string conversion logic into _format_message_for_text() helper function to improve readability and maintainability. Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com>

ayeshurun · 2025-12-30T19:03:12Z

src/fabric_cli/utils/fab_ui.py

+    # Get format from output or config, default to text
    format_type = output.output_format_type or fab_state_config.get_config(
        fab_constant.FAB_OUTPUT_FORMAT
-    )
+    ) or "text"


@copilot revert this change and if necessary pass --output_format json in the tests

Reverted the default fallback and added explicit output_format="text" to the test helper functions (mkdir, rm, import_cmd) instead. Changes in commit 6c4730e.

- Removed 'or "text"' fallback from print_output_format as requested - Added explicit output_format="text" to mkdir, rm, and import_cmd test helpers - This ensures tests work without relying on implicit defaults Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com>

aviatco · 2025-12-31T06:41:28Z

src/fabric_cli/utils/fab_ui.py

-    if output_result.message:
-        print_done(f"{output_result.message}\n")
+    if output_result.message is not None:
+        # Convert boolean messages to string for text output


@copilot remove the comment - this is clear from the code

Removed in commit 810a9ec.

tests/test_commands/conftest.py

aviatco · 2025-12-31T06:46:36Z

tests/test_commands/conftest.py

        command_path="rm",
        path=element_full_path,
        force=True,
+        output_format="text",


@copilot why you added output_format="text"? now it will override the format_type also when we set the config OUTPUT_FORMAT. I believe it will make some test's failure.

You're right - changed to use state_config.set_config(FAB_OUTPUT_FORMAT, "text") instead so it respects config overrides from fixtures. Fixed in commit 810a9ec.

aviatco · 2025-12-31T06:46:44Z

tests/test_commands/conftest.py

        input=content_path,
        force=True,
        format=format,
+        output_format="text",


@copilot why you added output_format="text"? now it will override the format_type also when we set the config OUTPUT_FORMAT. I believe it will make some test's failure.

You're right - changed to use state_config.set_config(FAB_OUTPUT_FORMAT, "text") instead so it respects config overrides from fixtures. Fixed in commit 810a9ec.

tests/test_commands/test_exists.py

aviatco · 2025-12-31T06:54:56Z

src/fabric_cli/core/fab_constant.py

 # Info
-INFO_EXISTS_TRUE = "true"
-INFO_EXISTS_FALSE = "false"
+INFO_EXISTS_TRUE = True


@ayeshurun If we removed the use of these constants in tests and the only place we use it is in exists command I think we can remove it and just return True/False - WDYT?

@copilot since the only place we use it is in exists command, and not in tests, please remove those constants and update the occurences to use True or False accordingly inline.

Removed the constants and replaced all usages with inline True and False in the exists command. Changes in commit f2080ba.

tests/test_commands/test_exists.py

tests/test_utils/test_fab_ui.py

aviatco · 2025-12-31T07:02:37Z

src/fabric_cli/core/fab_output.py


    @property
-    def message(self) -> Optional[str]:
+    def message(self) -> Optional[str | bool]:


@ayeshurun wouldn't be better approach to keep the login as is and only before printing the message in output format json convert the string boolean value? I believe this issue can accrue in other commands as well (can't think of an example now)

- Removed unnecessary comments that explain obvious code flow - Removed unused import (json) from test_exists.py - Changed test helpers to use state_config.set_config instead of args.output_format - This allows tests to override config via fixtures while helpers respect config - Cleaned up test comments that explain what's obvious from assertions Co-authored-by: aviatco <32952699+aviatco@users.noreply.github.com>

aviatco · 2025-12-31T17:17:07Z

tests/test_commands/conftest.py


 def mkdir(element_full_path, params=None):
    state_config.set_config(fab_constant.FAB_CACHE_ENABLED, "false")
+    state_config.set_config(fab_constant.FAB_OUTPUT_FORMAT, "text")


@copilot this is wrong. now any test who use this fixture will have FAB_OUTPUT_FORMAT value text, event if it was set in the test to another value. I think we should not have it here and if specific test need this configuration it should be at the test and not here.
Why did you add this?

Same for rm and import_cmd

You're absolutely right - removed the state_config.set_config(FAB_OUTPUT_FORMAT, "text") lines from all three helper functions. Instead, changed the setup_default_format fixture to class scope so it runs before class-scoped fixtures like workspace. Fixed in commit 50ec5b8.

- Removed state_config.set_config(FAB_OUTPUT_FORMAT, "text") from mkdir, rm, and import_cmd helpers - These helper functions should not set config that would override test-specific settings - Changed setup_default_format fixture to class scope and set config directly - This allows class-scoped workspace fixture to have format config available Co-authored-by: aviatco <32952699+aviatco@users.noreply.github.com>

- Removed INFO_EXISTS_TRUE and INFO_EXISTS_FALSE constants from fab_constant.py - Replaced usages with inline True and False in fab_fs_exists.py - Constants were only used in exists command, so inline booleans are cleaner Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com>

Initial plan

9dfa505

Copilot AI assigned Copilot and ayeshurun Dec 30, 2025

Copilot started work on behalf of ayeshurun December 30, 2025 18:40 View session

Copilot AI and others added 2 commits December 30, 2025 18:50

Refactor boolean message formatting into helper function

a073613

Extract inline boolean-to-string conversion logic into _format_message_for_text() helper function to improve readability and maintainability. Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix fab exists command to return boolean values~~ fix(exists): return boolean values in JSON output instead of strings Dec 30, 2025

Copilot finished work on behalf of ayeshurun December 30, 2025 18:55

Copilot AI requested a review from ayeshurun December 30, 2025 18:55

ayeshurun reviewed Dec 30, 2025

View reviewed changes

Copilot started work on behalf of ayeshurun December 30, 2025 19:03 View session

Copilot finished work on behalf of ayeshurun December 30, 2025 19:09

Copilot AI requested a review from ayeshurun December 30, 2025 19:09

aviatco reviewed Dec 31, 2025

View reviewed changes

Copilot started work on behalf of aviatco December 31, 2025 07:03 View session

Copilot finished work on behalf of aviatco December 31, 2025 07:09

Copilot AI requested a review from aviatco December 31, 2025 07:09

aviatco reviewed Dec 31, 2025

View reviewed changes

Copilot started work on behalf of aviatco December 31, 2025 17:17 View session

Copilot AI requested a review from aviatco December 31, 2025 17:21

Copilot finished work on behalf of aviatco December 31, 2025 17:21

Copilot started work on behalf of ayeshurun January 1, 2026 13:36 View session

Copilot finished work on behalf of ayeshurun January 1, 2026 13:40

fix(exists): return boolean values in JSON output instead of strings #108

Are you sure you want to change the base?

fix(exists): return boolean values in JSON output instead of strings #108

Uh oh!

Conversation

Copilot AI commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📥 Pull Request

✨ Description of new changes

Changes

Testing

fabric-cli version

Python version

Operating System

CLI Mode

Authentication Method

Steps to Reproduce

Expected Behavior

Actual Behavior

Additional Context

Possible Solution

Comments on the Issue (you are @copilot in this section)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Dec 30, 2025 •

edited

Loading