Revert "fix/fluxx malformed uri parsing"

[baharkyc]

Reverts #496

Summary by CodeRabbit

• Refactor

  • Optimized Fluxx URI instance extraction logic for improved code maintainability.

• Tests

  • Removed redundant URI parsing tests and updated test coverage.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR removes the extract_instance_from_uri helper function from the Fluxx module and replaces its usage with direct parsed_uri.hostname access. Associated test cases are also removed, simplifying the URI parsing logic across three files.

Changes

Cohort / File(s)	Summary
Fluxx helper removal ingestr/src/fluxx/helpers.py	Deleted extract_instance_from_uri(uri: str) function (25 lines) that previously parsed Fluxx URIs and extracted instance names, including handling for malformed URIs with schemes.
Test cleanup ingestr/src/fluxx/helpers_test.py	Removed imports of urlparse and related tests; deleted URI parsing and embedded-scheme test blocks (110 lines removed). Updated test_normalize_id_field_always_included to add test case for "acme.fluxx.io".
URI parsing simplification ingestr/src/sources.py	Replaced extract_instance_from_uri(uri) call with direct parsed_uri.hostname access for instance extraction, retaining ValueError if hostname is missing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• PR #496: Inverse operation that adds the extract_instance_from_uri helper and integrates it into sources.py—directly related refactoring.

Suggested reviewers

• albertobruin

Poem

🐰 A helper takes its final bow,
No more schemes in URIs now,
hostname does the job so clean,
Tests retired from the scene,
Simpler code, a rabbit's dream! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately reflects the primary change: reverting a previous fix for Fluxx malformed URI parsing, which aligns with the removal of the extract_instance_from_uri helper and related tests.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67319e7 and 314bcd7.

📒 Files selected for processing (3)

• ingestr/src/fluxx/helpers.py
• ingestr/src/fluxx/helpers_test.py
• ingestr/src/sources.py

💤 Files with no reviewable changes (1)

• ingestr/src/fluxx/helpers.py

🧰 Additional context used

🧬 Code graph analysis (1)

ingestr/src/fluxx/helpers_test.py (1)

ingestr/src/fluxx/helpers.py (2)

• _get_base_url (14-34)
• normalize_fluxx_item (186-233)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

• GitHub Check: tests (ubuntu-latest, 3.10)
• GitHub Check: tests (ubuntu-latest, 3.13)
• GitHub Check: tests (ubuntu-latest, 3.12)
• GitHub Check: tests (ubuntu-latest, 3.11)
• GitHub Check: Greptile Review

🔇 Additional comments (3)

ingestr/src/sources.py (1)

3571-3579: LGTM - Instance extraction reverted to direct hostname access.

The change correctly uses parsed_uri.hostname to extract the instance from URIs like fluxx://mycompany.preprod?client_id=xxx. Error handling for missing hostname is properly in place, and the downstream _get_base_url function handles the domain logic (appending .fluxxlabs.com for short names vs. using full domains with valid TLDs).

ingestr/src/fluxx/helpers_test.py (2)

13-13: LGTM - Import updated to reflect removed helper.

The import correctly reflects the removal of the extract_instance_from_uri helper function.

---

267-287: LGTM - Test coverage for _get_base_url is comprehensive.

The parametrized tests cover the key scenarios:

• Bare domains with valid TLDs (e.g., acme.fluxx.io)
• Domains already with .fluxxlabs.com suffix
• Short instance names that need the suffix appended
• Scheme preservation (http vs https)
• Preprod/subdomain instances

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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

[greptile-apps]

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

logic: reverting to parsed_uri.hostname will break URIs like fluxx://https://acme.fluxx.io - hostname will be parsed as https instead of the domain, causing API calls to fail

Prompt To Fix With AI

This is a comment left during a code review.
Path: ingestr/src/sources.py
Line: 3575:3575

Comment:
**logic:** reverting to `parsed_uri.hostname` will break URIs like `fluxx://https://acme.fluxx.io` - hostname will be parsed as `https` instead of the domain, causing API calls to fail

How can I resolve this? If you propose a fix, please make it concise.