Skip to content

Conversation

@lorchda
Copy link

@lorchda lorchda commented Nov 12, 2025

Issue #145

Description of changes:

For the configuration rvl-cdip-package-sample-with-few-shot-examples:

  • fix broken paths
  • fix classification prompt (add classes)
  • fix extraction prompt (add few-shot examples)

For few-shot example notebooks:

  • add AWS profile setup

For idp_common/extraction/service.py:

  • fix metering format

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@rstrahan rstrahan changed the base branch from main to develop November 12, 2025 14:18
Copy link
Contributor

@rstrahan rstrahan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much.. I left a couple of comments..
In general it will be much easier to understand and review if you can first create Issues to describe bugs/defects that you encounter, and then create a clean PR per issue to resolve.. This significantly limits the 'blast radius' of each PR, make is easier/safer to review, and we'll get it tested and merged faster.
Is that possible?

"output_tokens": 0,
"invocation_count": 0,
"total_cost": 0.0,
f"Extraction/{self.config.extraction.model}": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please report a separate Issue to clarify the problem that this is intended to fix? It's not clear to me that we have a bug in metering (looks OK in the UI and metering Athena table), so need clarity here.

"metadata": {},
"source": [
"## 1. Install Dependencies\n",
"## 1. Setup AWS Access"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again I am uncertain what issue this resolves.. and why it would be specific to the notebook? Any notebook should work already based on aws credential provider chain (typically local aws configure profile).. Can you log a separate issue if there is a bug, that this is intended to fix. Tx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants