Skip to content

Conversation

@algal
Copy link
Contributor

@algal algal commented Sep 9, 2025

This PR changes ContextKit to avoid from contextkit import * producing a name collision by importing its tool function read_url, which collides with SolveIt's built-in, automatically imported read_url tool function. It addresses #11

It produces the following effect:

  • adds a new function read_link which is equal to the old read_url and which is imported by import *
  • removes read_url from the import * list, but leaves it there so it can still be imported directly by name
  • prints a deprecation warning whenever someone calls read_url. (In a later version, we remove it entirely.)

This change breaks backward compatibility by removing read_url from the import * list. Any code depending on that line to get access to read_url, which upgrades to a version which includes this PR, will crash.

I'm very open to suggestions regarding ways to avoid this or handle the situation more graciously, while still achieving the goal of avoiding the name collision with SolveIt's read_url. I don't see any obvious choices.

This is to avoid a name collision with SolveIt's built-in read_url
tool function
@algal algal marked this pull request as draft September 9, 2025 19:52
This changes makes it so:

- `import *` now imports read_link
- `import *` no longer import read_url
- `read_url` can still be imported directly by name
- `read_url` prints a deprecation warning when called
@algal algal marked this pull request as ready for review September 9, 2025 20:07
@algal algal marked this pull request as draft September 9, 2025 20:11
@algal algal marked this pull request as ready for review September 9, 2025 20:32
@algal algal requested a review from johnowhitaker September 10, 2025 15:52
@algal
Copy link
Contributor Author

algal commented Sep 10, 2025

Hi @johnowhitaker I'd love to get your opinion on this!

Following up on our conversation this morning, maybe it would improve this PR to to modify either the documentation or also the default behavior of the new function read_link, so that it was easier to avoid downloading unwanted junk?

Right now, the signature and doctoring of read_link is just like read_url was, as follows:

def read_link(url: str,                  # URL to read
             heavy: bool = False,        # Use headless browser (requires extra setup steps before use)
             sel: Optional[str] = None,  # Css selector to pull content from
             useJina: bool = False,      # Use Jina for the markdown conversion
             ignore_links: bool = False, # Whether to keep links or not
             ): 
    "Reads a url and converts to markdown"
    ...

Would you suggest changing it to set useJina=True by default?

Or modifying the parameter doc comment by adding "AI guidance: set this to True for cleaner text"? Or putting that in the docstring? Or both?

Or both?

@jph00
Copy link
Contributor

jph00 commented Sep 11, 2025

lgtm! :)

@jph00 jph00 merged commit b4fc97d into main Sep 11, 2025
@jph00 jph00 added the enhancement New feature or request label Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants