feat: add knowledge CLI command group

[mdesmet]

Important

Adds a knowledge CLI command group to serve knowledge bases via an HTTP server, integrated into the datapilot CLI.

• CLI Command Group:
  • Adds knowledge command group in cli.py for knowledge base management.
  • serve command starts an HTTP server to serve knowledge bases.
• Server Implementation:
  • KnowledgeBaseHandler in server.py handles GET requests for /kb/{uuid} and /health.
  • Validates URL scheme and handles HTTP/HTTPS requests.
  • Logs messages using click.echo.
• Integration:
  • Integrates knowledge command into datapilot CLI in main.py.
  • Uses @click.version_option to display version information.

^{This description was created by for 5be69f7. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed f963721 in 1 minute and 20 seconds. Click for details.

• Reviewed 142 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/datapilot/cli/main.py:8

• Draft comment:
  Remove duplicate import of the knowledge CLI command and verify the import order to avoid unintended side effects.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

2. src/datapilot/core/knowledge/cli.py:67

• Draft comment:
  Cache the result of e.read() to avoid calling it twice, which may empty the stream.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_EW1I50EpfF4wDW0M

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to 2f3858c in 2 minutes and 0 seconds. Click for details.

• Reviewed 127 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/datapilot/cli/main.py:11

• Draft comment:
  Clear import alias for the knowledge CLI; ensure the parent context provides required config (token, instance_name).
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

2. src/datapilot/cli/main.py:91

• Draft comment:
  Adding the 'knowledge' command; verify that the configuration (token, instance_name, backend_url) is correctly set in the parent context.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

3. src/datapilot/core/knowledge/cli.py:35

• Draft comment:
  Consider using Python's uuid.UUID for validating the UUID instead of a regex for more robust handling.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_PLCZnfNdJKjLspdM

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 13b3d1b in 26 seconds. Click for details.

• Reviewed 14 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/datapilot/core/knowledge/cli.py:77

• Draft comment:
  Good refactor: assigning e.read() to error_body avoids reading the stream twice, which is both efficient and avoids potential issues with empty reads.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_AMVzIzjns2HsS1NQ

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 5222d4a in 39 seconds. Click for details.

• Reviewed 13 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/datapilot/core/knowledge/cli.py:67

• Draft comment:
  The '# noqa: S310' suppression is acceptable since the URL scheme is validated above. Consider adding a brief inline note explaining this rationale for future maintainers.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_3Ur4giaTKqYXkQOt

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[mdesmet]

We should maybe rename this server from knowledge to server as it may serve other purposes later.

currently this is invoked as

(.venv) ➜  datapilot-cli git:(feat/kb-proxy) datapilot knowledge serve
Starting knowledge base server on port 4000...
Backend URL: http://localhost:8000
Instance: gitpod
Server running at http://localhost:4000

[suryaiyer95]

Some comments

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed a02e6c2 in 46 seconds. Click for details.

• Reviewed 20 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/datapilot/cli/main.py:9

• Draft comment:
  Importing version from datapilot is correct for powering the --version option. Ensure version is maintained without causing circular import issues.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

2. src/datapilot/cli/main.py:55

• Draft comment:
  Adding the @click.version_option decorator is a good enhancement for CLI version reporting. Verify the decorator order works as expected with click.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_iciUVsn7qceviEYy

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 79efc66 in 63 minutes and 53 seconds. Click for details.

• Reviewed 31 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/datapilot/core/knowledge/cli.py:23

• Draft comment:
  Consider validating 'backend_url' along with 'token' and 'instance_name'. Currently, if 'backend_url' is missing or falsy, the handler will attempt to build an invalid URL.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

2. src/datapilot/core/knowledge/server.py:16

• Draft comment:
  Using class-level attributes for configuration (token, instance_name, backend_url) might lead to issues in concurrent scenarios. Consider using instance attributes if multi-threading is introduced.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_YaqJoOEFcHJVLTGc

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 5be69f7 in 1 minute and 11 seconds. Click for details.

• Reviewed 22 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/datapilot/core/knowledge/server.py:25

• Draft comment:
  Ensure clients update to the new '/kb/' route.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

2. src/datapilot/core/knowledge/server.py:37

• Draft comment:
  Verify backend endpoint change to '/knowledge_bases/private/' is intentional.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment violates our rules in multiple ways. First, it starts with "Verify that..." which is explicitly called out as not useful. Second, it's asking the author to confirm their intention, which we're told not to do. Third, we should assume the author made this change intentionally - if they didn't, it would be caught in testing. Maybe this is a security-sensitive change that really does need extra verification? Changing from public to private endpoints could have security implications. While security is important, we should trust that the author tested their changes. If this was a security issue, it would be caught in the review process and testing. Asking for verification adds no value. Delete this comment as it violates our rules by asking for verification of an intentional change.

3. src/datapilot/core/knowledge/server.py:24

• Draft comment:
  The comment still refers to '/knowledge_bases/{uuid}' even though the regex pattern was updated to '/kb/{uuid}'. Please update the comment for consistency.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_rtTRv9gjJByjgPnv

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}