n8n UI Customisations & Translation Infrastructure#6
n8n UI Customisations & Translation Infrastructure#6apl-intento wants to merge 52 commits intomasterfrom
Conversation
delete condition for building only amd64
disable login to docker hub
…y Assurance subcategories to AI nodes
… attempts and delay
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive translation infrastructure for n8n with custom Intento packages. The implementation includes agent-based architecture, supplier pattern, ICU-based segmentation, DeepL translation integration, and extensive test coverage.
Changes:
- Added translation infrastructure with
@intento/translationpackage including contexts, suppliers (DeepL, DryRun), and request/response patterns - Implemented segmentation infrastructure with
@intento/segmentationpackage featuring EchoGarden ICU-based text splitting - Created n8n node integration layer in
@intento/nodes-basewith tools for translation and segmentation - Added comprehensive test suites with 95%+ coverage targets
- Fixed test infrastructure issues in
@n8n/node-cliincluding environment variable handling and file cleanup delays
Reviewed changes
Copilot reviewed 197 out of 321 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/@n8n/nodes-langchain/nodes/ToolExecutor/ToolExecutor.node.json | Updated subcategory from "Helpers" to "Primitives" |
| packages/@n8n/node-cli/src/utils/package-manager.test.ts | Added missing tmpdir fixture parameter |
| packages/@n8n/node-cli/src/test-utils/temp-fs.ts | Added 100ms delay before cleanup to prevent file handle issues |
| packages/@n8n/node-cli/src/commands/*.test.ts | Set npm_config_user_agent for package manager detection |
| packages/@n8n/eslint-config/tsconfig.json | Removed empty files array |
| packages/@intento/translation/* | Complete translation package with contexts, suppliers, types |
| packages/@intento/segmentation/* | Complete segmentation package with ICU-based text splitting |
| packages/@intento/nodes-base/* | n8n node implementations for translation and segmentation tools |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| jobs: | ||
| build-arm64: | ||
| runs-on: ubuntu-22.04-arm | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
ARM64 build job runs on x86_64 runner
High Severity
The build-arm64 job's runner was changed from ubuntu-22.04-arm to ubuntu-latest. The job is explicitly named "build-arm64" with a step called "Setup and Build ARM64", but ubuntu-latest is an x86_64 runner, not ARM64. This means the job will not produce ARM64 artifacts as intended. The publish-to-docker-hub job depends on this build completing successfully, so release builds expecting ARM64 binaries will either fail or receive x86_64 binaries incorrectly.
| this.dryRunContext = ContextFactory.read<DryRunContext>(DryRunContext, this.functions, this.tracer); | ||
| this.splitContext = ContextFactory.read<SplitContext>(SplitContext, this.functions, this.tracer); | ||
| this.descriptor.batchLimit = this.splitContext.batchSize; | ||
| this.descriptor.segmentLimit = this.splitContext.segmentSize; |
There was a problem hiding this comment.
Shared descriptor object mutated causing cross-instance interference
Medium Severity
The DryRunSupplier constructor mutates this.descriptor.batchLimit and this.descriptor.segmentLimit directly. Since DryRunDescriptor is a shared exported constant and TranslationSupplierBase stores the descriptor reference without copying (this.descriptor = descriptor), all DryRunSupplier instances share the same descriptor object. When multiple instances are created with different splitContext configurations, each instance overwrites the shared descriptor's values, causing all instances to use the last-configured values instead of their own. This leads to incorrect batching behavior when multiple DryRunSuppliers are used in the same workflow.
Additional Locations (1)
…-8-n-ui-customisations
| description: "Docker image to use (for docker-pull mode)" | ||
| required: false | ||
| default: "n8nio/n8n:nightly" | ||
| default: "n8nio/n8n:nightly" |
There was a problem hiding this comment.
Duplicate YAML keys break workflow input definitions
High Severity
Multiple workflow input fields have duplicate YAML keys for description and default values. For example, branch, test-mode, test-command, shards, and docker-image inputs each have their description and default lines appearing twice. Additionally, runs-on: ubuntu-latest appears twice at lines 70-71. YAML parsers may fail or silently use only the last value, causing unexpected behavior in CI.
Additional Locations (1)
| needs: determine-build-context | ||
| runs-on: ${{ matrix.runner }} | ||
| timeout-minutes: 30 | ||
| timeout-minutes: 30 |
There was a problem hiding this comment.
Duplicate timeout-minutes YAML key in workflow
Medium Severity
The build-and-push-docker job has timeout-minutes: 30 specified twice on consecutive lines (86 and 87). Duplicate YAML keys are invalid and may cause parsing issues or unpredictable behavior depending on the YAML parser used.
| this.to = context.to; | ||
| this.from = context.from; | ||
|
|
||
| Object.freeze(this); |
| const splitResult = await this.splitText(request, signal); | ||
| if (splitResult instanceof SupplyError) return new AgentError(request, splitResult.code, splitResult.reason); |
There was a problem hiding this comment.
Is it a common pattern to return an error instead of throwing an exception?
| private segmentation?: SegmentsSupplierBase; | ||
| private translation?: TranslationSupplierBase[]; |
There was a problem hiding this comment.
let's rename them to something more precise, like translationTools and segmentationTools
| } | ||
|
|
||
| protected async execute(request: TranslationAgentRequest, signal: AbortSignal): Promise<TranslationAgentResponse | AgentError> { | ||
| signal.throwIfAborted(); |
There was a problem hiding this comment.
I think it is better to add event listener once then check aborted it or not synchoniusly in each function many times
| if (result instanceof TranslationResponse) return result; | ||
| if (i >= this.translation!.length - 1) return result; | ||
| } | ||
| throw new Error('No translation suppliers were found.'); |
There was a problem hiding this comment.
It could be that translation just failed on each tool
| * @template TS - Response type extending AgentResponseBase with latency tracking | ||
| */ | ||
| export abstract class AgentBase<TI extends AgentRequestBase, TS extends AgentResponseBase> { | ||
| private _initialize: Promise<void> | null = null; |
There was a problem hiding this comment.
What is the point of having this field? I do not see any use of it
| try { | ||
| tracer.debug(`Fetching node parameter '${key}' for context '${context}' ...`); | ||
| // NOTE: extractValue resolves expressions and extracts from collection/fixedCollection parameters | ||
| const value = functions.getNodeParameter(key, 0, undefined, { extractValue: true }); |
| // NOTE: Base delay scaled so exponential growth reaches maxDelayMs at final retry | ||
| const baseDelay = this.maxDelayMs / 2 ** (this.maxAttempts - 1); | ||
| // NOTE: Jitter uses uniform random variance to prevent thundering herd | ||
| const jitter = this.maxJitter * (Math.random() - 0.5) * 2 * baseDelay; |
There was a problem hiding this comment.
jitter can be negative. Is it expected?
| */ | ||
| protected onError(request: TI, error: Error): SupplyError { | ||
| // NOTE: TimeoutError from AbortSignal.timeout() - execution exceeded configured limit | ||
| if (error instanceof DOMException && error.name === 'TimeoutError') { |
There was a problem hiding this comment.
Isn't DOMException related to the frontend part? If not, what is under the DOM abbreviation?
| constructor(descriptor: IDescriptor, connection: IntentoConnectionType, functions: IFunctions) { | ||
| this.connection = connection; | ||
| this.functions = functions; | ||
| this.tracer = new Tracer(descriptor, functions); | ||
| this.descriptor = descriptor; | ||
| this.executionContext = ContextFactory.read<ExecutionContext>(ExecutionContext, functions, this.tracer); | ||
| } |
There was a problem hiding this comment.
``
| constructor(descriptor: IDescriptor, connection: IntentoConnectionType, functions: IFunctions) { | |
| this.connection = connection; | |
| this.functions = functions; | |
| this.tracer = new Tracer(descriptor, functions); | |
| this.descriptor = descriptor; | |
| this.executionContext = ContextFactory.read<ExecutionContext>(ExecutionContext, functions, this.tracer); | |
| } | |
| constructor( | |
| readonly descriptor: IDescriptor, | |
| protected readonly connection: IntentoConnectionType, | |
| protected readonly functions: IFunctions) { | |
| this.tracer = new Tracer(descriptor, functions); | |
| this.executionContext = ContextFactory.read<ExecutionContext>(ExecutionContext, functions, this.tracer); | |
| } |
And you can delete definition in 22-24 rows
| * @param reason - Human-readable error description for n8n display | ||
| * @param isRetriable - Whether operation can be retried (false for permanent failures) | ||
| */ | ||
| constructor(request: SupplyRequestBase, code: number, reason: string, isRetriable: boolean) { |
There was a problem hiding this comment.
simplify class definition how I show in supplier-base.ts file comment
| if (!this.agentRequestId || this.agentRequestId.trim() === '') throw new Error('"agentRequestId" is required'); | ||
| if (!this.supplyRequestId || this.supplyRequestId.trim() === '') throw new Error('"supplyRequestId" is required'); | ||
| if (!this.reason || this.reason.trim() === '') throw new Error('"reason" is required'); | ||
| if (this.code < 100 || this.code > 599) throw new RangeError('"code" must be a valid HTTP status code (100-599)'); |
| * Used by @mapTo decorator to attach parameter key names to constructor parameters | ||
| * via reflect-metadata, enabling automatic extraction from n8n node parameters. | ||
| */ | ||
| export const CONTEXT_PARAMETER = Symbol('intento:context_parameter'); |
There was a problem hiding this comment.
|
|
||
| signal?.throwIfAborted(); | ||
|
|
||
| return await new Promise((resolve, reject) => { |
There was a problem hiding this comment.
return await make sense only in try catch blocks. You can just return promise and function will become sync
| * RegExpValidator.isValidPattern("//") // false - empty pattern | ||
| * RegExpValidator.isValidPattern("/[/") // false - invalid regex | ||
| */ | ||
| static isValidPattern(pattern: string): boolean { |
There was a problem hiding this comment.
I don't get this function, there are valid patterns but they will fail match check.
| protected async execute(request: TranslationRequest, signal: AbortSignal): Promise<TranslationResponse | SupplyError> { | ||
| signal.throwIfAborted(); | ||
|
|
||
| const from = !request.from || request.from === '' ? 'auto' : request.from.toLowerCase(); |
There was a problem hiding this comment.
| const from = !request.from || request.from === '' ? 'auto' : request.from.toLowerCase(); | |
| const from = request.from?.toLowerCase() || 'auto' |
3 participants
📋 Overview
Major feature addition implementing a complete translation infrastructure for n8n with custom Intento packages, including agent-based architecture, supplier pattern, segmentation capabilities, and comprehensive test coverage.
🎯 Key Features
1. Agent-Based Architecture (
@intento/agents)2. Core Infrastructure (
@intento/core)@mapTo3. Segmentation Package (
@intento/segmentation)4. Translation Package (
@intento/translation)throwIfInvalid()pattern5. n8n Node Integration (
@intento/nodes-base)Note
Introduces Intento translation stack and streamlines CI/tooling.
packages/@intento/core(agent framework:AgentBase,AgentRequest/Response,AgentError,ContextFactorywith@mapTo,ExecutionContext,Tracer) with extensive unit tests and Jest/ESLint configspackages/@intento/agents(initialTranslationAgent+ request/response/descriptor wiring) and project scaffoldingubuntu-latestrunners; minor Docker manifest cleanup; Playwright/unit/lint jobs aligned; Codecov remainsprintWidthto 140, add.vscodelaunch, gitignorelogs/, andstart:devscriptWritten by Cursor Bugbot for commit 33e2bdd. This will update automatically on new commits. Configure here.