Decorators fixes latest #19

andreibratu · 2025-02-27T15:04:49Z

Functions wrapped in HL decorators enforce receiving functions that take inputs and optionally, a message argument
Functions wrapped in HL decorators keep their signature
Use /otel in processing
prompt decorator no longer creates its own span; similar to Python, we add metadata directly on LLM provider span
Validation and better warnings on evaluations run

andreibratu · 2025-03-06T12:48:08Z

package.json

    "repository": "https://github.com/humanloop/humanloop-node",
-    "main": "./index.js",
-    "types": "./index.d.ts",
+    "main": "./dist/index.js",


revert before merging

harry-humanloop · 2025-03-07T18:29:39Z

package.json

+        "qs": "6.11.2",
+        "readable-stream": "^4.5.2",
+        "stable-hash": "0.0.4",
+        "url-join": "4.0.1"


any of these ESM-only? (have we tested it's installable on customer-like environments that had issue with pmap?)

those are deps packed by fern

harry-humanloop · 2025-03-07T18:30:25Z

src/context.ts

+    }
+}
+
+// ... existing code ...


i assume this is AI?

(is this a sign that any other bits of code need cleanup/looking at?)

harry-humanloop · 2025-03-07T18:49:29Z

src/decorators/tool.ts

+ *
+ * @param func - The function to wrap
+ * @param opentelemetryTracer - The OpenTelemetry tracer instance
+ * @param path - Optional span path


is this optional? path below is a non-optional string

harry-humanloop · 2025-03-07T18:50:16Z

src/decorators/tool.ts

+    }
+
+    Object.keys(inputs!).forEach((inputKey) => {
+        if (!parameters.hasOwnProperty(inputKey)) {


is this the right way to check, or is "in" better?

jamesbaskerville

generally lgtm but needs some cleanup. utilities/ should be empty.

jamesbaskerville · 2025-03-07T18:37:39Z

src/otel/exporter.ts

+                .filter(([_, value]) => value !== undefined)
+                .map(([key, value]) => ({
+                    key,
+                    value: { stringValue: value!.toString() },


same comment as python -- does string always make sense? Do we not send any other type of attr?

outdated & fixed

jamesbaskerville · 2025-03-07T18:38:51Z

src/otel/exporter.ts

+                    "X-Fern-SDK-Name": "humanloop",
+                    "X-Fern-SDK-Version": SDK_VERSION,
+                },
+                body: JSON.stringify(this.spanToProto(span)),


question: Does this match the OTLP spec for JSON spans? Slightly concerned there are differences that are unknown. We may be better off directly sending protos around and processing them as such.

scoped out https://linear.app/humanloop/issue/ENG-1706/use-protos-instead-of-custom-ts-interface-for-export

jamesbaskerville · 2025-03-07T18:42:12Z

src/humanloop.client.ts

+        OpenAI?: any,
+        Anthropic?: any,
+        CohereAI?: any,


why this change ooc?

jamesbaskerville · 2025-03-07T18:43:34Z

src/humanloop.client.ts

-        version,
-    }: {
-        callable: InputsMessagesCallableType<I, M, O>;
+    public prompt<I, O>(args: {


These three should probably still have docstrings, though they can be a little less involved than previously

jamesbaskerville · 2025-03-07T18:48:21Z

src/decorators/flow.ts

+                                        "content" in funcOutput
+                                    ) {
+                                        outputMessage =
+                                            funcOutput as unknown as ChatMessage;


jamesbaskerville · 2025-03-07T19:01:05Z

src/evals/run.ts

+    console.log(`\n${CYAN}Navigate to your Evaluation:${RESET}\n${evaluation.url}\n`);
+    console.log(
+        `${CYAN}${type_.charAt(0).toUpperCase() + type_.slice(1)} Version ID: ${
+            hlFile.versionId
+        }${RESET}`,
+    );
+    console.log(`${CYAN}Run ID: ${runId}${RESET}`);


Why are these in console logs instead of process.stdout.write?

jamesbaskerville · 2025-03-07T19:02:15Z

src/evals/run.ts

+        // TODO: trigger run when updated API is available
+        console.log(
+            `${CYAN}\nRunning ${hlFile.name} over the Dataset ${hlDataset.name}${RESET}`,
+        );


so does this do nothing atm? What does this TODO mean?

not sure, pre my changes

jamesbaskerville · 2025-03-07T19:02:47Z

src/evals/run.ts

+            await new Promise((resolve) => setTimeout(resolve, 500));
+        }
+    } while (stats.status !== "completed");
+    console.log(stats.report);


same q on process.stdout vs. console.log

jamesbaskerville · 2025-03-07T19:03:08Z

src/evals/run.ts

+                // TODO: Add back in with number valence on Evaluators
+                // improvementCheck: improvementCheck,


does this need to go in now?

jamesbaskerville · 2025-03-07T19:04:52Z

src/evals/run.ts

+        });
+    } catch (error: any) {
+        // If the name exists, go and get it
+        // TODO: Update API GET to allow querying by name and file.


is this a linear ticket?

added https://linear.app/humanloop/issue/ENG-1705/allow-querying-file-by-name-path-for-sdk

andreibratu commented Mar 6, 2025

View reviewed changes

harry-humanloop reviewed Mar 7, 2025

View reviewed changes

jamesbaskerville approved these changes Mar 7, 2025

View reviewed changes

andreibratu force-pushed the decorators-fixes-latest branch from df194c1 to 2838674 Compare March 10, 2025 11:22

Andrei Bratu added 2 commits March 10, 2025 12:52

remove tests from fernignore

4fc66ca

rebase changes

dfe5cc9

andreibratu force-pushed the decorators-fixes-latest branch from f663e59 to dfe5cc9 Compare March 10, 2025 12:55

api errors are easier to parse

ea16dfe

andreibratu marked this pull request as ready for review March 10, 2025 13:27

andreibratu merged commit 392886c into master Mar 10, 2025
3 checks passed

		// TODO: Add back in with number valence on Evaluators
		// improvementCheck: improvementCheck,

Decorators fixes latest #19

Decorators fixes latest #19

Uh oh!

Conversation

andreibratu commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

jamesbaskerville left a comment

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!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andreibratu commented Feb 27, 2025 •

edited

Loading