-
Notifications
You must be signed in to change notification settings - Fork 94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: project governance custom config #1504
base: master
Are you sure you want to change the base?
Changes from 15 commits
016e33e
5fec9ea
ed44ada
4e0404f
767a2ed
01d3c9d
f9bcb77
f6344bc
4c144f7
1b18a21
028d5be
6934b15
58b0109
f45b9c4
f38dcd3
aec0f0f
3f0e632
0e2599c
e4a7034
6aee129
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
from typing import Optional | ||
|
||
def project_healthcheck( | ||
manifest_path, catalog_path=None, config_path=None, config=None | ||
manifest_path, catalog_path=None, config_path=None, config=None, token=None, tenant=None, backend_url: Optional[str] = None, | ||
): | ||
try: | ||
import logging | ||
|
@@ -20,6 +22,9 @@ def project_healthcheck( | |
manifest=manifest, | ||
catalog=catalog, | ||
config=config, | ||
token=token, | ||
instance_name=tenant, | ||
backend_url=backend_url, | ||
Comment on lines
+25
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add parameter validation and consider renaming for consistency
Consider adding validation: + if backend_url and not backend_url.startswith(('http://', 'https://')):
+ raise ValueError("backend_url must be a valid HTTP(S) URL")
+ if token and not isinstance(token, str):
+ raise ValueError("token must be a string")
insight_generator = DBTInsightGenerator(
manifest=manifest,
catalog=catalog,
config=config,
token=token,
instance_name=tenant,
backend_url=backend_url,
)
|
||
) | ||
reports = insight_generator.run() | ||
|
||
|
@@ -28,6 +33,8 @@ def project_healthcheck( | |
k: [json.loads(item.json()) for item in v] | ||
for k, v in reports[MODEL].items() | ||
} | ||
|
||
return {"model_insights": model_insights} | ||
|
||
except Exception as e: | ||
raise Exception(str(e)) |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -334,7 +334,7 @@ export interface ConversationGroup { | |||||
|
||||||
@provideSingleton(AltimateRequest) | ||||||
export class AltimateRequest { | ||||||
private static ALTIMATE_URL = workspace | ||||||
public static ALTIMATE_URL = workspace | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing
Suggested change
|
||||||
.getConfiguration("dbt") | ||||||
.get<string>("altimateUrl", "https://api.myaltimate.com"); | ||||||
Comment on lines
+338
to
340
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add URL validation for workspace configuration. The URL from workspace configuration is used without validation. Consider adding URL validation to prevent potential security issues: public static ALTIMATE_URL = workspace
.getConfiguration("dbt")
- .get<string>("altimateUrl", "https://api.myaltimate.com");
+ .get<string>("altimateUrl", "https://api.myaltimate.com");
+ private static validateUrl(url: string): string {
+ try {
+ const parsedUrl = new URL(url);
+ if (!['http:', 'https:'].includes(parsedUrl.protocol)) {
+ throw new Error('Invalid protocol');
+ }
+ return url;
+ } catch (e) {
+ console.warn(`Invalid URL in configuration: ${url}, using default`);
+ return "https://api.myaltimate.com";
+ }
+ }
|
||||||
|
||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -1155,7 +1155,7 @@ export class DBTCoreProjectIntegration | |||||||||||||||
await healthCheckThread.ex`from dbt_healthcheck import *`; | ||||||||||||||||
const result = await healthCheckThread.lock<ProjectHealthcheck>( | ||||||||||||||||
(python) => | ||||||||||||||||
python!`to_dict(project_healthcheck(${manifestPath}, ${catalogPath}, ${configPath}, ${config}))`, | ||||||||||||||||
python!`to_dict(project_healthcheck(${manifestPath}, ${catalogPath}, ${configPath}, ${config}, ${this.altimateRequest.getAIKey()}, ${this.altimateRequest.getInstanceName()}, ${AltimateRequest.ALTIMATE_URL}))`, | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add validation for authentication parameters. The code passes authentication parameters directly to Apply this diff to add validation: - python!`to_dict(project_healthcheck(${manifestPath}, ${catalogPath}, ${configPath}, ${config}, ${this.altimateRequest.getAIKey()}, ${this.altimateRequest.getInstanceName()}, ${AltimateRequest.ALTIMATE_URL}))`,
+ const aiKey = this.altimateRequest.getAIKey();
+ const instanceName = this.altimateRequest.getInstanceName();
+ if (!aiKey || !instanceName) {
+ throw new Error('Missing required authentication parameters for health check.');
+ }
+ python!`to_dict(project_healthcheck(${manifestPath}, ${catalogPath}, ${configPath}, ${config}, ${aiKey}, ${instanceName}, ${AltimateRequest.ALTIMATE_URL}))`, 📝 Committable suggestion
Suggested change
|
||||||||||||||||
); | ||||||||||||||||
return result; | ||||||||||||||||
} finally { | ||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -561,27 +561,33 @@ export class InsightsPanel extends AltimateWebviewProvider { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
args: JSON.stringify(args), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
await window.withProgress( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
title: `Performing healthcheck...`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
location: ProgressLocation.Notification, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
cancellable: false, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
const projectHealthcheck = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
await this.dbtProjectContainer.executeAltimateDatapilotHealthcheck( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
args, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._panel!.webview.postMessage({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
command: "response", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
args: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
syncRequestId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
body: { projectHealthcheck }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
status: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
window.showInformationMessage( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
"Started performing healthcheck. We will notify you once it's done.", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
const projectHealthcheck = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
await this.dbtProjectContainer.executeAltimateDatapilotHealthcheck( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
args, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+568
to
+571
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Error handling needs improvement in the healthcheck execution chain
Add proper error handling in try {
window.showInformationMessage("Started performing healthcheck. We will notify you once it's done.");
const projectHealthcheck = await this.dbtProjectContainer.executeAltimateDatapilotHealthcheck(args);
// ... success handling ...
} catch (error) {
window.showErrorMessage(`Healthcheck failed: ${error.message}`);
this.dbtTerminal.error("InsightsPanel", "Healthcheck execution failed", error);
} 🔗 Analysis chainVerify error handling in executeAltimateDatapilotHealthcheck. The health check execution might fail for various reasons (network issues, invalid configuration, etc.). Ensure comprehensive error handling in the called method. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check error handling in executeAltimateDatapilotHealthcheck implementation
# Search for the method implementation
ast-grep --pattern 'executeAltimateDatapilotHealthcheck(args$_) {
$$$
}'
# Search for error handling patterns
rg -A 5 'catch|throw|Error' src/dbt_client/dbtProjectContainer.ts
Length of output: 222 Script: #!/bin/bash
# Let's first find the correct file containing the implementation
fd "dbtProjectContainer"
# Then search for the method and error handling patterns in the found files
rg -l "executeAltimateDatapilotHealthcheck"
# Once we find the files, we'll examine the implementation and error handling
Length of output: 178 Script: #!/bin/bash
# Now that we found the correct files, let's examine the implementation and error handling
ast-grep --pattern 'executeAltimateDatapilotHealthcheck($_) {
$$$
}'
# Also check for any try-catch blocks or error handling around this method
rg -B 2 -A 10 "executeAltimateDatapilotHealthcheck" src/manifest/dbtProjectContainer.ts
# Look for error handling patterns in the implementation
rg -A 5 'catch|throw|Error' src/manifest/dbtProjectContainer.ts
Length of output: 1048 Script: #!/bin/bash
# Let's check the implementation of performDatapilotHealthcheck in the project class
rg -B 2 -A 10 "performDatapilotHealthcheck"
# Also check for error handling in the calling code in insightsPanel.ts
rg -B 2 -A 10 "executeAltimateDatapilotHealthcheck" src/webview_provider/insightsPanel.ts
Length of output: 6320 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
const result = await window.showInformationMessage( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
"Healthcheck completed successfully.", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
"View results", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (result === "View results") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.dbtTerminal.debug( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
"InsightsPanel", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
"Sending healthcheck results to webview", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
await commands.executeCommand("dbtPowerUser.Insights.focus"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._panel!.webview.postMessage({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
command: "response", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
args: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
syncRequestId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
body: { projectHealthcheck }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
status: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+595
to
+602
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider adding error handling for webview communication. The webview message sending is not wrapped in a try-catch block, which could lead to unhandled exceptions if the webview is closed. Add error handling: - this._panel!.webview.postMessage({
- command: "response",
- args: {
- syncRequestId,
- body: { projectHealthcheck },
- status: true,
- },
- });
+ try {
+ if (this._panel) {
+ await this._panel.webview.postMessage({
+ command: "response",
+ args: {
+ syncRequestId,
+ body: { projectHealthcheck },
+ status: true,
+ },
+ });
+ }
+ } catch (error) {
+ this.dbtTerminal.error(
+ "InsightsPanel",
+ "Failed to send healthcheck results to webview",
+ error
+ );
+ } 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
} catch (e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.emitError( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
syncRequestId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ import { ButtonHTMLAttributes } from 'react'; | |
import { ButtonProps } from 'reactstrap'; | ||
import { CaseReducerActions } from '@reduxjs/toolkit'; | ||
import { ChatMessage } from '@ant-design/pro-chat'; | ||
import { ComponentType } from 'react'; | ||
import { Dispatch } from 'react'; | ||
import { JSX as JSX_2 } from 'react/jsx-runtime'; | ||
import { PayloadAction } from '@reduxjs/toolkit'; | ||
|
@@ -66,9 +67,16 @@ export declare interface CoachAiResponse { | |
personalizationScope: string; | ||
} | ||
|
||
export declare const CoachForm: ({ taskLabel, context, onClose, extra }: Props_10) => JSX_2.Element; | ||
export declare const CoachForm: (props: CoachFormProps) => JSX_2.Element; | ||
|
||
export declare const CoachFormButton: ({}: Props_11) => JSX_2.Element; | ||
export declare const CoachFormButton: ({}: Props_10) => JSX_2.Element; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove empty interface for CoachFormButton props The empty interface Apply this diff: -declare interface Props_10 {
-}
-export declare const CoachFormButton: ({}: Props_10) => JSX_2.Element;
+export declare const CoachFormButton: () => JSX_2.Element; Also applies to: 350-351 🧰 Tools🪛 Biome[error] 72-72: Unexpected empty object pattern. (lint/correctness/noEmptyPattern) |
||
|
||
declare interface CoachFormProps { | ||
taskLabel: keyof typeof TaskLabels; | ||
context?: Record<string, unknown>; | ||
extra?: Record<string, unknown>; | ||
onClose: (data?: unknown) => void; | ||
} | ||
|
||
export declare const CodeBlock: ({ code, language, fileName, editorTheme, theme, showLineNumbers, className, titleActions, }: Props_4) => JSX.Element; | ||
|
||
|
@@ -206,7 +214,7 @@ export declare const IconButton: (props: Props) => JSX.Element; | |
export declare interface Learning extends z.infer<typeof learningSchema> { | ||
} | ||
|
||
export declare const Learnings: ({ filters, learning }: Props_12) => JSX_2.Element; | ||
export declare const Learnings: ({ filters, learning }: Props_11) => JSX_2.Element; | ||
|
||
export declare const learningSchema: z.ZodObject<{ | ||
train_doc_uid: z.ZodString; | ||
|
@@ -282,28 +290,74 @@ export declare enum PersonalizationScope { | |
ALL_USERS = "AllUsers" | ||
} | ||
|
||
export declare enum ProjectGovernorAllowedFiles { | ||
Manifest = "Manifest", | ||
Catalog = "Catalog" | ||
} | ||
|
||
export declare interface ProjectGovernorCheck extends z.infer<typeof ProjectGovernorCheckSchema> { | ||
id: string; | ||
} | ||
|
||
export declare interface ProjectGovernorCheckConfirmationResponse { | ||
ok: boolean; | ||
} | ||
|
||
export declare interface ProjectGovernorCheckFormValues { | ||
description: string; | ||
type: ProjectGovernorCheckTypes; | ||
} | ||
|
||
export declare const ProjectGovernorCheckSchema: z.ZodObject<{ | ||
name: z.ZodString; | ||
alias: z.ZodString; | ||
type: z.ZodEnum<[string, ...string[]]>; | ||
description: z.ZodString; | ||
files_required: z.ZodArray<z.ZodEnum<[string, ...string[]]>, "many">; | ||
config: z.ZodRecord<z.ZodString, z.ZodUnknown>; | ||
}, "strip", z.ZodTypeAny, { | ||
type: string; | ||
name: string; | ||
description: string; | ||
alias: string; | ||
files_required: string[]; | ||
config: Record<string, unknown>; | ||
}, { | ||
type: string; | ||
name: string; | ||
description: string; | ||
alias: string; | ||
files_required: string[]; | ||
config: Record<string, unknown>; | ||
}>; | ||
|
||
export declare enum ProjectGovernorCheckTypes { | ||
DOCUMENTATION = "documentation", | ||
TESTS = "tests", | ||
MODEL = "model", | ||
FILE_STRUCTURE = "file_structure" | ||
} | ||
|
||
export declare interface ProjectGovernorCheckValidateResponse { | ||
name: string; | ||
description: string; | ||
} | ||
|
||
declare interface Props extends ButtonHTMLAttributes<HTMLButtonElement> { | ||
color?: string; | ||
} | ||
|
||
declare interface Props_10 { | ||
taskLabel: keyof typeof TaskLabels; | ||
context?: Record<string, unknown>; | ||
extra?: Record<string, unknown>; | ||
onClose: () => void; | ||
} | ||
|
||
declare interface Props_11 { | ||
} | ||
|
||
declare interface Props_12 { | ||
filters?: { | ||
taskLabel?: keyof typeof TaskLabels; | ||
}; | ||
learning?: string | null; | ||
} | ||
|
||
declare interface Props_13 { | ||
declare interface Props_12 { | ||
onSelect: (selected: (typeof TeamMatesConfig)[0], action: TeamMateActionType) => Promise<boolean | undefined>; | ||
client: keyof typeof TeamMateAvailability; | ||
} | ||
|
@@ -437,7 +491,8 @@ export declare enum TaskLabels { | |
DocGen = "DocGen", | ||
ChartBot = "ChartBot", | ||
SqlBot = "SqlExpert", | ||
OpportunitiesBot = "OpportunitiesBot" | ||
OpportunitiesBot = "OpportunitiesBot", | ||
ProjectGovernor = "ProjectGovernor" | ||
} | ||
|
||
export declare const TeammateActions: CaseReducerActions< { | ||
|
@@ -463,6 +518,8 @@ export declare interface TeamMateConfig { | |
key: TaskLabels; | ||
seeInAction?: boolean; | ||
comingSoon?: boolean; | ||
displayComponent?: ComponentType<any>; | ||
formComponent?: ComponentType<any>; | ||
Comment on lines
+521
to
+522
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve type safety for TeamMateConfig component properties The Apply this diff: - displayComponent?: ComponentType<any>;
- formComponent?: ComponentType<any>;
+ displayComponent?: ComponentType<TeamMateDisplayProps>;
+ formComponent?: ComponentType<TeamMateFormProps>;
+interface TeamMateDisplayProps {
+ // Add specific props for display component
+}
+
+interface TeamMateFormProps {
+ // Add specific props for form component
+}
|
||
} | ||
|
||
export declare interface TeamMateContextProps { | ||
|
@@ -474,7 +531,7 @@ export declare const TeamMateProvider: ({ children, }: { | |
children: ReactNode; | ||
}) => JSX.Element; | ||
|
||
export declare const TeamMates: ({ onSelect, client }: Props_13) => JSX_2.Element; | ||
export declare const TeamMates: ({ onSelect, client }: Props_12) => JSX_2.Element; | ||
|
||
export declare const TeamMatesConfig: TeamMateConfig[]; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,34 +1,37 @@ | ||
import { A as o, B as t, m as n, p as i, o as r, q as C, n as m, t as T, v as l, C as c, F as p, k as g, i as v, h as L, D as u, I as h, y as B, l as M, L as d, P as A, G as b, K as k, J as y, r as F, z as P, E as x, x as D, T as I, H as S, w as f } from "./main.js"; | ||
import { A as s, B as t, m as n, p as r, o as i, q as C, n as c, t as m, v as l, C as T, F as p, k as v, i as g, h, D as L, I as u, y as M, l as P, L as d, P as k, N as A, O as B, M as y, G as F, K as G, J as b, r as S, z as j, E as x, x as D, T as I, H as f, w } from "./main.js"; | ||
import "reactstrap"; | ||
export { | ||
o as ApiHelper, | ||
s as ApiHelper, | ||
t as Badge, | ||
n as CLL, | ||
i as ChatTriggerLink, | ||
r as Chatbot, | ||
r as ChatTriggerLink, | ||
i as Chatbot, | ||
C as Citations, | ||
m as CllEvents, | ||
T as CoachForm, | ||
c as CllEvents, | ||
m as CoachForm, | ||
l as CoachFormButton, | ||
c as CodeBlock, | ||
T as CodeBlock, | ||
p as ContentCategory, | ||
g as ConversationGroupProvider, | ||
v as ConversationInputForm, | ||
L as ConversationSources, | ||
u as DbtDocs, | ||
h as IconButton, | ||
B as Learnings, | ||
M as Lineage, | ||
v as ConversationGroupProvider, | ||
g as ConversationInputForm, | ||
h as ConversationSources, | ||
L as DbtDocs, | ||
u as IconButton, | ||
M as Learnings, | ||
P as Lineage, | ||
d as LoadingButton, | ||
A as PersonalizationScope, | ||
b as TaskLabels, | ||
k as TeamMateActionType, | ||
y as TeamMateAvailability, | ||
F as TeamMateProvider, | ||
P as TeamMates, | ||
k as PersonalizationScope, | ||
A as ProjectGovernorAllowedFiles, | ||
B as ProjectGovernorCheckSchema, | ||
y as ProjectGovernorCheckTypes, | ||
F as TaskLabels, | ||
G as TeamMateActionType, | ||
b as TeamMateAvailability, | ||
S as TeamMateProvider, | ||
j as TeamMates, | ||
x as TeamMatesConfig, | ||
D as TeammateActions, | ||
I as Tooltip, | ||
S as learningSchema, | ||
f as useTeamMateContext | ||
f as learningSchema, | ||
w as useTeamMateContext | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add type hints and improve parameter documentation
The function signature could benefit from:
Consider applying this improvement:
📝 Committable suggestion