sync.py: allow check_file to fail

[aesteve-rh]

Summary by Sourcery

Bug Fixes:

• Handle invalid or unexpected status text from git results by defaulting the sync status to success instead of failing.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

check_file now parses the git command output into a SyncStatus and can surface non-success statuses instead of always reporting success.

Sequence diagram for updated check_file status handling

sequenceDiagram
    participant Caller
    participant SyncService
    participant GitBackend
    participant SyncStatus
    participant SyncResult

    Caller->>SyncService: check_file(remote_name, file_sync)
    SyncService->>GitBackend: run_git_check(remote_name, file_sync)
    GitBackend-->>SyncService: git_result(message, objects_processed)

    SyncService->>SyncStatus: from_text(git_result.message)
    alt message maps to known status
        SyncStatus-->>SyncService: status
    else message does not map (ValueError)
        SyncStatus--xSyncService: ValueError
        SyncService->>SyncService: status = SyncStatus.SUCCESS
    end

    SyncService->>SyncResult: create(file_sync, remote_name, status, git_result.message, git_result.objects_processed, local_hash=None)
    SyncService-->>Caller: SyncResult

Loading

Class diagram for SyncStatus and SyncResult usage in check_file

classDiagram
    class SyncService {
        +check_file(remote_name: str, file_sync: FileSync) SyncResult
    }

    class FileSync {
        +include: str
        +exclude: str
    }

    class SyncStatus {
        <<enumeration>>
        SUCCESS
        FAILURE
        WARNING
        +from_text(message: str) SyncStatus
    }

    class SyncResult {
        +file_sync: FileSync
        +remote_name: str
        +status: SyncStatus
        +message: str
        +files_processed: int
        +local_hash: str
    }

    SyncService --> FileSync : uses
    SyncService --> SyncResult : returns
    SyncService --> SyncStatus : parses
    SyncResult --> FileSync : contains
    SyncResult --> SyncStatus : has_status

Loading

File-Level Changes

Change	Details	Files
Propagate SyncStatus parsed from git_result.message instead of hard-coding success, defaulting to success on parse failure.	Wrap SyncStatus.from_text(git_result.message) in a try/except to handle ValueError when the output cannot be parsed Use the parsed SyncStatus value when constructing SyncResult instead of always using SyncStatus.SUCCESS Fallback to SyncStatus.SUCCESS when the git_result.message cannot be parsed into a SyncStatus	src/git_crossref/sync.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• Catching a broad ValueError and defaulting to SUCCESS risks silently masking unexpected parsing issues; consider logging the exception or at least the unparseable message to make those cases visible during debugging.
• If SyncStatus.from_text can fail for known non-success states (e.g., malformed but clearly indicating failure), you might want to distinguish between an unknown status (default to SUCCESS) and a clearly non-success indication to avoid treating genuine failures as successes.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Catching a broad ValueError and defaulting to SUCCESS risks silently masking unexpected parsing issues; consider logging the exception or at least the unparseable message to make those cases visible during debugging.
- If SyncStatus.from_text can fail for known non-success states (e.g., malformed but clearly indicating failure), you might want to distinguish between an unknown status (default to SUCCESS) and a clearly non-success indication to avoid treating genuine failures as successes.

## Individual Comments

### Comment 1
<location> `src/git_crossref/sync.py:121` </location>
<code_context>
+            try:
+                status = SyncStatus.from_text(git_result.message)
+            except ValueError:
+                status = SyncStatus.SUCCESS
+
             return SyncResult(
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Defaulting to SUCCESS on parse failure may mask unexpected or malformed status messages.

If `SyncStatus.from_text` raises `ValueError`, defaulting to `SUCCESS` can hide real failures or new status types. Prefer a neutral/explicit fallback (e.g., `UNKNOWN` or `ERROR`), or propagate the parsing failure so unexpected formats don’t get reported as successful.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}