Feature/knowledge connector s3 #831

tmccaughey · 2025-12-11T13:56:32Z

Hi Falak. I have fixed the chunking size, it is not exactly identical to the one in the manual upload but a lot better. Thanks for your time! :)

Copilot

Pull request overview

This PR adds an S3 knowledge connector feature to the AWS extension, enabling extraction and processing of documents from S3 buckets into knowledge sources. It also updates deprecated APIs and dependencies.

Key changes:

Implements S3 connector with support for multiple file types (txt, pdf, docx, csv, json, jsonl, md, pptx)
Integrates LangChain for document loading and text chunking with configurable chunk sizes
Updates deprecated Buffer constructor and context API methods

Reviewed changes

Copilot reviewed 15 out of 17 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
extensions/aws/tsconfig.json	Added compiler options for better module compatibility
extensions/aws/src/nodes/lambdaInvoke.ts	Fixed deprecated Buffer constructor and updated context API call
extensions/aws/src/module.ts	Registered S3 connector and commented out existing nodes
extensions/aws/src/knowledge-connectors/s3Connector.ts	Main connector implementation for processing S3 files into knowledge sources
extensions/aws/src/knowledge-connectors/helpers/text_extractor.ts	Document loader using LangChain for various file types
extensions/aws/src/knowledge-connectors/helpers/text_chunker.ts	Text splitting logic with configurable chunk sizes
extensions/aws/src/knowledge-connectors/helpers/new_utils.ts	S3 file download and chunk extraction utilities
extensions/aws/src/knowledge-connectors/helpers/list_files.ts	S3 bucket listing functionality
extensions/aws/src/knowledge-connectors/helpers/utils/*.ts	Utility functions for text processing and configuration
extensions/aws/src/knowledge-connectors/helpers/creds.env	Environment variable template for AWS credentials
extensions/aws/package.json	Updated dependencies to support LangChain and newer AWS SDK
extensions/aws/.npmrc	Added legacy peer deps flag for dependency resolution

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-11T13:58:02Z

extensions/aws/src/module.ts

+		// sayPollyNode,
+		// lambdaInvokeNode,
+		// s3GetObjectNode,
+		// s3PutObjectNode


All existing nodes have been commented out, leaving the nodes array empty. This breaks existing functionality for users of this extension. If these nodes are being temporarily disabled for testing, consider using a feature flag instead, or create a separate branch for the S3 connector work that doesn't disable production features.

Suggested change

// sayPollyNode,

// lambdaInvokeNode,

// s3GetObjectNode,

// s3PutObjectNode

sayPollyNode,

lambdaInvokeNode,

s3GetObjectNode,

s3PutObjectNode

Copilot · 2025-12-11T13:58:02Z

extensions/aws/src/knowledge-connectors/helpers/text_extractor.ts

+};
+
+/**
+ * Custom PPTXLoader class to handle pptx files. Impelementation adapted


Corrected spelling of 'Impelementation' to 'Implementation'.

Suggested change

* Custom PPTXLoader class to handle pptx files. Impelementation adapted

* Custom PPTXLoader class to handle pptx files. Implementation adapted

Copilot · 2025-12-11T13:58:03Z

extensions/aws/src/knowledge-connectors/helpers/text_chunker.ts

+	return chunkSize;
+};
+
+	const getRecursiveCharacterTextSplitter = () => {


Incorrect indentation - this function definition has an extra tab character at the beginning of the line. The indentation should align with other top-level function definitions in the file.

Suggested change

const getRecursiveCharacterTextSplitter = () => {

const getRecursiveCharacterTextSplitter = () => {

Copilot · 2025-12-11T13:58:03Z

extensions/aws/src/knowledge-connectors/helpers/text_chunker.ts

+	});
+	return splitter;
+};
+	export const splitTextUsingRecursiveCharacterTextSplitter = (text: string) => {


Incorrect indentation - this exported function has an extra tab character at the beginning of the line. The indentation should align with other top-level exports in the file.

Suggested change

export const splitTextUsingRecursiveCharacterTextSplitter = (text: string) => {

export const splitTextUsingRecursiveCharacterTextSplitter = (text: string) => {

Copilot · 2025-12-11T13:58:04Z

extensions/aws/src/knowledge-connectors/helpers/list_files.ts

+import { get } from "http";
+
+


The 'get' import from 'http' is unused in this file and should be removed.

Suggested change

import { get } from "http";

Copilot · 2025-12-11T13:58:04Z

extensions/aws/src/knowledge-connectors/helpers/list_files.ts

@@ -0,0 +1,67 @@
+import { S3Client, ListObjectsV2Command, S3 } from "@aws-sdk/client-s3";


The 'S3' import from '@aws-sdk/client-s3' is unused in this file and should be removed.

Suggested change

import { S3Client, ListObjectsV2Command, S3 } from "@aws-sdk/client-s3";

import { S3Client, ListObjectsV2Command } from "@aws-sdk/client-s3";

Copilot · 2025-12-11T13:58:04Z

extensions/aws/src/knowledge-connectors/helpers/creds.env

+AWS_ACCESS_KEY_ID=your_access_key_here
+AWS_SECRET_ACCESS_KEY=your_secret_key_here
+AWS_REGION=eu-central-1
+BUCKET_NAME=your_bucket_name_here


Environment credential files should not be committed to the repository. This file should be added to .gitignore and replaced with a .env.example or .env.template file to document required variables without exposing placeholder values that could be mistaken for real credentials.

Do we need this file?

falak-asad · 2026-01-07T11:51:24Z

extensions/aws/src/knowledge-connectors/helpers/creds.env

+AWS_ACCESS_KEY_ID=your_access_key_here
+AWS_SECRET_ACCESS_KEY=your_secret_key_here
+AWS_REGION=eu-central-1
+BUCKET_NAME=your_bucket_name_here


Do we need this file?

falak-asad · 2026-01-07T11:51:59Z

extensions/aws/src/knowledge-connectors/helpers/text_chunker.ts

+			});
+			break;
+
+		case "RecursiveCharacterTextSplitter":


We can just use RecursiveCharacterTextSplitter for all and remove option to switch beween splitters, to make the code simplified

falak-asad · 2026-01-07T11:53:31Z

extensions/aws/src/knowledge-connectors/s3Connector.ts

+      return supportedExtensions.some(ext => keyLower.endsWith(`.${ext}`));
+    });
+
+    console.log(`Filtered to ${filteredObject.length} supported files (skipped ${s3Objects.length - filteredObject.length} other files)`);


Do we need to add so many console.logs? We can just add the comments instead of logs

falak-asad · 2026-01-07T11:55:04Z

extensions/aws/src/knowledge-connectors/helpers/text_chunker.ts

+	const splitter = getRecursiveCharacterTextSplitter();
+
+	// Override separators to use default ones, to ensure chunk size limits are respected
+	splitter.separators = ["\n\n", "\n", " ", ""];


We can just use default sperator, did you try default seperator and what are the results? I think adding "", will cause chunks to be quite small.

falak-asad · 2026-01-07T11:56:27Z

extensions/aws/src/knowledge-connectors/helpers/text_chunker.ts

+	return splitParagraphs;
+}
+
+const getChunkSizeInTokens = () => {


If we keep only RecursiveCharacterTextSplitter, then this function is not needed

falak-asad · 2026-01-07T11:58:07Z

extensions/aws/src/knowledge-connectors/helpers/utils/removeUnnecessaryChars.ts

@@ -0,0 +1,13 @@
+export const removeUnnecessaryChars = (text: string): string => {


we can move the utils like removeUnnecessaryChars and convertToUtf8.ts to text_extractor.ts as its only used there, no need to create separate files.

falak-asad · 2026-01-07T11:59:57Z

extensions/aws/src/knowledge-connectors/helpers/new_utils.ts

@@ -0,0 +1,97 @@
+import type { IKnowledge } from "@cognigy/extension-tools";


file name new_utils should be renamed to something better.

falak-asad · 2026-01-07T12:03:45Z

Suggested changes:

We need to minimize the code, and things that we are not using, should be removed
Fix some indentation issues, that copilot suggested in the review
Remove unnecessary files
Remove console logs and replace it with comments.

tmccaughey added 3 commits November 17, 2025 16:45

added first version

4361527

set limit to 500 and fixed sync

11c958e

Fixed chunk size

ea67374

tmccaughey requested review from Copilot and falak-asad December 11, 2025 13:56

Copilot AI reviewed Dec 11, 2025

View reviewed changes

falak-asad reviewed Jan 7, 2026

View reviewed changes

	* Custom PPTXLoader class to handle pptx files. Impelementation adapted
	* Custom PPTXLoader class to handle pptx files. Implementation adapted

	const getRecursiveCharacterTextSplitter = () => {
	const getRecursiveCharacterTextSplitter = () => {

	export const splitTextUsingRecursiveCharacterTextSplitter = (text: string) => {
	export const splitTextUsingRecursiveCharacterTextSplitter = (text: string) => {

		@@ -0,0 +1,67 @@
		import { S3Client, ListObjectsV2Command, S3 } from "@aws-sdk/client-s3";

	import { S3Client, ListObjectsV2Command, S3 } from "@aws-sdk/client-s3";
	import { S3Client, ListObjectsV2Command } from "@aws-sdk/client-s3";

		@@ -0,0 +1,13 @@
		export const removeUnnecessaryChars = (text: string): string => {

		@@ -0,0 +1,97 @@
		import type { IKnowledge } from "@cognigy/extension-tools";

Feature/knowledge connector s3 #831

Are you sure you want to change the base?

Feature/knowledge connector s3 #831

Uh oh!

Conversation

tmccaughey commented Dec 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

falak-asad commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants