simulate: condense verbose comments

Author: theomonnom

cmd/lk/simulate.go

@@ -88,9 +88,7 @@ var simulateCommand = &cli.Command{
 }
 
 // writeGeneratedScenariosTemp writes a generated run's scenarios to a temp
-// scenarios.yaml and returns its path, so the simulate command can print where
-// they landed (mirroring how it prints the agent log path). Returns "" when the
-// run carries no generated scenarios.
+// scenarios.yaml; "" when the run carries none.
 func writeGeneratedScenariosTemp(run *livekit.SimulationRun) (string, error) {
 	group := run.GetScenarioGroup()
 	if group == nil || len(group.GetScenarios()) == 0 {
@@ -137,9 +135,8 @@ func scenarioGroupToYAML(group *livekit.ScenarioGroup) ([]byte, error) {
 	return yaml.Marshal(f)
 }
 
-// scenariosFile mirrors a scenarios.yaml (the source of truth for scenarios).
-// It maps field-for-field onto livekit.ScenarioGroup; `userdata` is written as a
-// nested mapping here and JSON-encoded into the proto's string field.
+// scenariosFile mirrors a scenarios.yaml; `userdata` is a nested mapping here
+// and JSON-encoded into the proto's string field.
 type scenariosFile struct {
 	Name      string         `yaml:"name"`
 	Scenarios []yamlScenario `yaml:"scenarios"`
@@ -153,7 +150,6 @@ type yamlScenario struct {
 	Userdata          map[string]any    `yaml:"userdata"`
 }
 
-// simulateConfig holds all parameters needed to run a simulation in either TUI or CI mode.
 type simulateConfig struct {
 	ctx            context.Context
 	client         *lksdk.AgentSimulationClient
@@ -169,16 +165,13 @@ type simulateConfig struct {
 	scenariosPath  string // path to the --scenarios file (empty when generating from source)
 }
 
-// simulateMode represents how scenarios are sourced.
 type simulateMode int
 
 const (
 	modeScenarios simulateMode = iota
 	modeGenerateFromSource
 )
 
-// loadScenarioGroup reads a scenarios.yaml into a livekit.ScenarioGroup, JSON-encoding
-// each scenario's nested `userdata` mapping into the proto's string field.
 func loadScenarioGroup(path string) (*livekit.ScenarioGroup, error) {
 	data, err := os.ReadFile(path)
 	if err != nil {
@@ -239,9 +232,8 @@ func runSimulate(ctx context.Context, cmd *cli.Command) error {
 		return err
 	}
 
-	// The scenarios file must be specified explicitly via --scenarios; we never
-	// auto-discover one. When provided, those scenarios are the source of truth;
-	// otherwise scenarios are generated from the agent's source.
+	// never auto-discovered: an explicit --scenarios file is the source of
+	// truth, otherwise scenarios are generated from the agent's source
 	scenariosPath := cmd.String("scenarios")
 
 	var scenarioGroup *livekit.ScenarioGroup
@@ -259,8 +251,6 @@ func runSimulate(ctx context.Context, cmd *cli.Command) error {
 		mode = modeGenerateFromSource
 	}
 
-	// Generating from source uploads the agent's code to LiveKit Cloud, so make
-	// the user agree to it explicitly before anything is sent.
 	if mode == modeGenerateFromSource {
 		if err := confirmSourceUpload(cmd, projectDir); err != nil {
 			return err
@@ -297,8 +287,8 @@ func isInteractive() bool {
 	return isatty.IsTerminal(os.Stdin.Fd()) && isatty.IsTerminal(os.Stdout.Fd())
 }
 
-// confirmSourceUpload makes the user explicitly agree that their agent's source
-// code will be uploaded to LiveKit Cloud before generating scenarios from it.
+// confirmSourceUpload makes the user agree before their agent's source is
+// uploaded to LiveKit Cloud.
 func confirmSourceUpload(cmd *cli.Command, projectDir string) error {
 	if cmd.Bool("yes") {
 		return nil
@@ -333,9 +323,9 @@ func confirmSourceUpload(cmd *cli.Command, projectDir string) error {
 
 // --- Shared lifecycle functions used by both TUI and CI modes ---
 
-// agentLauncher owns the agent subprocess lifecycle around the TUI, which only
-// observes the start via Wait. Stop kills the worker even when the TUI quits
-// mid-start; a leaked worker keeps its port bound and breaks the next run.
+// agentLauncher owns the agent subprocess lifecycle around the TUI. Stop kills
+// the worker even when the TUI quits mid-start; a leaked worker keeps its port
+// bound and breaks the next run.
 type agentLauncher struct {
 	done chan struct{}
 	proc *AgentProcess
@@ -351,14 +341,13 @@ func launchSimulationAgent(c *simulateConfig) *agentLauncher {
 	return l
 }
 
-// Wait blocks until the start attempt finishes and returns its result.
 func (l *agentLauncher) Wait() (*AgentProcess, error) {
 	<-l.done
 	return l.proc, l.err
 }
 
-// Stop kills the agent once the start attempt finishes (bounded so a stuck
-// start can't hang the exit path) and returns it for post-exit reporting.
+// Stop kills the agent once the start attempt finishes (bounded wait) and
+// returns it for post-exit reporting.
 func (l *agentLauncher) Stop() *AgentProcess {
 	select {
 	case <-l.done:
@@ -395,9 +384,8 @@ func startSimulationAgent(c *simulateConfig, forwardOutput io.Writer) (*AgentPro
 			"--simulation",
 		},
 		Env: []string{
-			// force the agent to register under the dispatch name regardless of any
-			// agent_name hardcoded in the user's code (see LIVEKIT_AGENT_NAME_OVERRIDE
-			// precedence in livekit-agents worker.py).
+			// register under the dispatch name regardless of any agent_name
+			// hardcoded in the user's code
 			"LIVEKIT_AGENT_NAME_OVERRIDE=" + c.agentName,
 			"LIVEKIT_URL=" + c.pc.URL,
 			"LIVEKIT_API_KEY=" + c.pc.APIKey,
@@ -417,8 +405,6 @@ func createSimulationRun(ctx context.Context, c *simulateConfig) (string, *livek
 		req.Concurrency = &c.concurrency
 	}
 	if c.mode == modeScenarios {
-		// Run the scenarios from the yaml. When unset, the server generates
-		// num_simulations scenarios from the uploaded source.
 		req.ScenarioGroup = c.scenarioGroup
 	}
 

cmd/lk/simulate_ci.go

@@ -144,8 +144,7 @@ func runSimulateCI(ctx context.Context, config *simulateConfig) error {
 			}
 			fmt.Fprintf(os.Stderr, "Warning: poll failed: %v\n", err)
 		} else {
-			// The worker is failing systemically (crashing on job startup, never
-			// joining), not failing a scenario. Stop early and surface its log.
+			// the worker is failing systemically: stop early and surface its log
 			if !brokenAgent && agentBroken(run, agent) {
 				brokenAgent = true
 				report.BrokenAgent()
@@ -197,8 +196,8 @@ func runSimulateCI(ctx context.Context, config *simulateConfig) error {
 	return nil
 }
 
-// writeRunResults writes the per-job results and the run summary, using GitHub
-// Actions group markers (harmless and a useful delimiter outside GitHub too).
+// writeRunResults writes the per-job results and the run summary, with GitHub
+// group markers (a useful delimiter outside GitHub too).
 func writeRunResults(w io.Writer, run *livekit.SimulationRun, ap *AgentProcess) {
 	if run == nil {
 		return

cmd/lk/simulate_health.go

@@ -20,12 +20,9 @@ import (
 	"github.com/livekit/protocol/livekit"
 )
 
-// A simulation can stall because the agent worker is broken (crashed on startup,
-// never joins a room) rather than because a scenario failed. When that happens
-// the CLI cancels the run instead of waiting out a 30s timeout per scenario, and
-// surfaces the worker's own error. The detection has to separate a broken agent
-// from transient connection pacing, which can cause an isolated "no agent joined"
-// timeout even when the agent is healthy.
+// Detects a broken agent worker (crashed on startup, never joins) so the CLI
+// can cancel the run and surface the worker's error, while tolerating
+// transient "no agent joined" timeouts from connection pacing.
 
 // pythonFatalMarkers are livekit-agents (Python) log lines that only appear on
 // a fatal, non-transient worker error. The JS framework will need its own set.
@@ -35,17 +32,14 @@ var pythonFatalMarkers = []string{
 	"closing due to unrecoverable error",
 }
 
-// minFatalMarkers is how many fatal log lines it takes to call the worker
-// broken; a single one can be an isolated job crash.
+// a single fatal log line can be an isolated job crash
 const minFatalMarkers = 2
 
-// maxAgentNotJoined is how many "no agent joined" timeouts to tolerate before
-// treating the worker as broken.
+// "no agent joined" timeouts tolerated before the worker counts as broken
 const maxAgentNotJoined = 3
 
 // agentBroken reports whether the worker is failing systemically. A completed
-// scenario proves the agent works, so it is never broken in that case; otherwise
-// repeated fatal log markers or "no agent joined" timeouts mark it broken.
+// scenario proves the agent works.
 func agentBroken(run *livekit.SimulationRun, ap *AgentProcess) bool {
 	completed, notJoined := 0, 0
 	for _, job := range run.GetJobs() {
@@ -68,9 +62,8 @@ func agentBroken(run *livekit.SimulationRun, ap *AgentProcess) bool {
 	}
 }
 
-// agentErrorContext is the worker output to surface for a broken agent: the whole
-// block from the last fatal marker to the end of the log, so the full traceback
-// survives, or the recent tail when no marker is present.
+// agentErrorContext is the worker output to surface for a broken agent: from
+// the last fatal marker to the end (full traceback), or the recent tail.
 func agentErrorContext(ap *AgentProcess) []string {
 	logs := ap.RecentLogs(0)
 	if i := lastFatalMarker(logs); i >= 0 {
@@ -85,8 +78,6 @@ func agentErrorContext(ap *AgentProcess) []string {
 	return out
 }
 
-// lastFatalMarker returns the index of the last log line matching a fatal
-// marker, or -1 if none.
 func lastFatalMarker(logs []string) int {
 	last := -1
 	for i, line := range logs {

cmd/lk/simulate_report.go

@@ -24,9 +24,8 @@ import (
 	"github.com/livekit/protocol/livekit"
 )
 
-// simLog is the single source of the plain-text simulation output: CI writes
-// it to the terminal (results on out, transient progress on info) and the
-// TUI's runReporter writes the same calls to the report file.
+// simLog is the single source of the plain-text simulation output, shared by
+// the CI mode (terminal) and the TUI's runReporter (report file).
 type simLog struct {
 	out        io.Writer
 	info       io.Writer
@@ -82,7 +81,6 @@ func (l *simLog) RunCreated(runID, dashboardURL string) {
 	fmt.Fprintln(l.out)
 }
 
-// RunUpdate logs run-status transitions and job progress from a poll result.
 func (l *simLog) RunUpdate(run *livekit.SimulationRun, configuredN int32) {
 	_, done, _, _ := simulationJobCounts(run)
 	switch run.Status {
@@ -129,8 +127,8 @@ func writeBrokenAgentNote(w io.Writer, ap *AgentProcess) {
 	}
 }
 
-// asciiWriter transliterates the output glyphs to plain ASCII, keeping the
-// report file free of special characters without forking the simLog strings.
+// asciiWriter keeps the report file free of special characters without
+// forking the simLog strings.
 type asciiWriter struct{ w io.Writer }
 
 var asciiGlyphs = strings.NewReplacer(
@@ -162,7 +160,6 @@ func newRunReporter() *runReporter {
 	return &runReporter{simLog: newSimLog(w, w), f: f}
 }
 
-// Finish appends the results and trailer, closes the file, returns its path.
 func (r *runReporter) Finish(run *livekit.SimulationRun, ap *AgentProcess, brokenAgent bool, dashboardURL string) string {
 	if r.f == nil {
 		return ""

cmd/lk/simulate_subprocess.go

@@ -138,8 +138,8 @@ type AgentStartConfig struct {
 // start/dev/console/simulate subcommands under `python -m livekit.agents`.
 const thinCLIMinVersion = "1.6.0"
 
-// agentExitDetail surfaces the agent's own output (the real error) and a
-// pointer to the full log, for when the worker exits early or never registers.
+// agentExitDetail surfaces the agent's own output and the log path when the
+// worker exits early or never registers.
 func agentExitDetail(ap *AgentProcess) string {
 	var b strings.Builder
 	if tail := lastNonEmptyLines(ap.RecentLogs(0), 12); len(tail) > 0 {
@@ -175,20 +175,16 @@ func startAgent(cfg AgentStartConfig) (*AgentProcess, error) {
 		return nil, err
 	}
 
-	// Reuse the SDK-version reader (parses the project's deps) to fail fast with a
-	// friendly message when livekit-agents is older than the thin-CLI baseline.
+	// fail fast when livekit-agents is older than the thin-CLI baseline
 	if err := agentfs.CheckSDKVersion(cfg.Dir, cfg.ProjectType, map[string]string{
 		"python-min-sdk-version": thinCLIMinVersion,
 		"node-min-sdk-version":   thinCLIMinVersion,
 	}); err != nil {
 		return nil, err
 	}
 
-	// Launch via the framework CLI module rather than running the user's file
-	// directly: python -m livekit.agents SUBCOMMAND ENTRYPOINT FLAGS. The framework
-	// discovers the AgentServer from the entrypoint and drives the thin CLI. Requires a
-	// livekit-agents that supports start/console under -m livekit.agents; older versions
-	// only expose download-files there.
+	// python -m livekit.agents SUBCOMMAND ENTRYPOINT FLAGS: the framework
+	// discovers the AgentServer from the entrypoint and drives the thin CLI.
 	args := append(prefixArgs, "-m", "livekit.agents")
 	if len(cfg.CLIArgs) > 0 {
 		args = append(args, cfg.CLIArgs[0]) // subcommand: start | console
@@ -393,9 +389,9 @@ func extractLogRoom(line string) string {
 	return ""
 }
 
-// Kill terminates the worker quickly: a short SIGINT grace (an idle worker
-// exits cleanly; one draining jobs would take minutes), then SIGKILL to the
-// whole process group, waiting for the exit so the port is free on return.
+// Kill gives the worker a short SIGINT grace (an idle one exits cleanly; one
+// draining jobs would take minutes), then SIGKILLs the whole process group and
+// waits so the port is free on return.
 func (ap *AgentProcess) Kill() {
 	if ap.cmd.Process == nil {
 		return