Skip to content

feat: observability (token usage, run log, acceptance metrics) - #66

Open
Benkapner wants to merge 3 commits into
mainfrom
feat/observability
Open

feat: observability (token usage, run log, acceptance metrics)#66
Benkapner wants to merge 3 commits into
mainfrom
feat/observability

Conversation

@Benkapner

Copy link
Copy Markdown
Collaborator

Summary

Adds instrumentation so users can see what the tool did, what it cost, and whether its suggestions are useful. Generated content is unchanged.

  • Token usage tracking (src/telemetry.py): thread-safe accumulator for prompt/completion tokens per stage. Collapsed <details> block appended to the update-mode confirmation comment. Optional cost-per-1m-input / cost-per-1m-output inputs for estimated cost; when unset, shows token counts only. Backends without usage data show "not reported".
  • Structured run log (src/run_log.py): JSONL with one record per LLM call (timestamp, stage, file, prompt/response length, usage, latency, outcome). Full prompt/response text behind opt-in debug-artifacts input. Sanitized through sanitize_output().
  • Acceptance rate: on [update-docs], computes suggested vs accepted from the prior review's checkbox state. Emits to run output and as an acceptance-rate Action output. Local only.

Test plan

  • uv run pytest -v passes (428 tests)
  • uv run ruff check src/ tests/ and uv run ruff format --check src/ tests/ clean
  • Token usage summary renders correctly in a test run
  • Run log JSONL is parseable and contains expected fields

Users have no idea what a run costs. With parallel generation plus
verification, a large PR fans out to dozens of LLM calls invisibly.
Add a UsageTracker that accumulates prompt/completion tokens per call
tagged by stage, and appends a collapsed token usage summary to the
update-mode confirmation comment. Optional cost-per-1m-input and
cost-per-1m-output action inputs enable estimated cost reporting;
when unset, only token counts are shown. Backends that omit usage
show "not reported" rather than zero.
Debugging a bad generation currently means reading interleaved print()
output from parallel threads, with prompts unrecoverable. Add a JSONL
run log with one record per LLM call: timestamp, stage, file path,
prompt/response length, token usage, latency, outcome. Behind an
opt-in debug-artifacts input, include full prompt and response text.
All output is routed through sanitize_output() so credentials cannot
leak into an artifact.
The checkbox UI already generates the project's quality signal: how
many suggested files a human leaves checked. On [update-docs], parse
the prior review comment's checkbox state and compute suggested vs
accepted. Emit the ratio into the run output and as an Action output.
Does not send anything off-repo.
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 17, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:35 AM UTC · Completed 5:53 AM UTC

Commit: 1255ba3 · View workflow run →

@Benkapner
Benkapner requested a review from csoceanu August 17, 2026 05:36
@Benkapner Benkapner self-assigned this Aug 17, 2026
@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

High

  • [dead-code] src/suggest_docs.pyRunLog is instantiated but its record() method is never called anywhere in the codebase. RunLog.record() requires 7 arguments (stage, file_path, prompt, response_text, usage_obj, latency_ms, outcome), but no call site is wired up in generation.py or elsewhere. As a result, run_log.has_entries will always be False and the "Run log written to" message will never print. The entire RunLog feature is inert.
    Remediation: Wire up run_log.record() calls at each LLM call site in generation.py alongside the existing usage_tracker.record() calls, or defer RunLog instantiation to a follow-up PR.

  • [api-contract] action.yml:95 — The action declares an acceptance-rate output, but entrypoint.sh never writes acceptance-rate=... to $GITHUB_OUTPUT. The acceptance rate is only printed to stdout in suggest_docs.py. Downstream workflows referencing steps.<id>.outputs.acceptance-rate will always get an empty string.
    Remediation: Write the acceptance rate to GITHUB_OUTPUT in suggest_docs.py (e.g., append acceptance-rate=<value> to the file at os.environ.get('GITHUB_OUTPUT')).

  • [missing-doc] CLAUDE.md:79 — The environment variables table is missing three new variables added in action.yml: COST_PER_1M_INPUT, COST_PER_1M_OUTPUT, and DEBUG_ARTIFACTS.
    Remediation: Add three rows to the environment variables table with descriptions matching action.yml.

  • [missing-doc] README.md:199 — The Optional Action Inputs section lists only style-config-path but is missing the three new inputs: cost-per-1m-input, cost-per-1m-output, and debug-artifacts.
    Remediation: Add documentation for the three new inputs to the Optional Action Inputs section.

Medium

  • [missing-authorization] action.yml — This PR introduces 3 new inputs, 1 new output, and 2 new modules (~180 lines) but has no linked issue authorizing this scope of work. The PR body clearly describes the motivation but a linked issue would provide authorization traceability.

  • [error-handling-gap] src/suggest_docs.py:253float(cost_input) will crash with ValueError on non-numeric input (e.g., $3.00). The existing get_max_context_chars() in config.py uses try/except with a warning and fallback — this should follow the same pattern.

  • [missing-doc] CLAUDE.md:24 — The source modules table is missing two new modules: telemetry.py and run_log.py.

  • [missing-doc] README.md — No action outputs are documented anywhere. The PR adds acceptance-rate but the three pre-existing outputs (status, modified-files, pr-created) are also absent.

Low

  • [data-exposure] src/run_log.py — When debug-artifacts is enabled, full prompts containing code diffs are written to the log. Currently inert (record() never called), but the infrastructure is in place. Consider adding a warning to the debug-artifacts input description.

  • [missing-doc] README.md:171 — Workflow example does not include the new optional inputs (consistent with existing omissions of other optional inputs).

  • [naming-convention] src/run_log.pyrecord() method with 7 parameters lacks Args documentation. Some codebase modules (discovery.py, comments.py) use structured Args/Returns docstrings for complex methods.

  • [naming-convention] src/telemetry.pyrecord() and format_summary() docstrings lack Args/Returns sections.

  • [logic-error] src/telemetry.py:89 — Total row in format_summary() shows all calls via len(self._records) but only sums tokens from stages that reported usage, making totals misleading when some stages lack usage data.

  • [edge-case] src/telemetry.py:93 — Cost condition uses truthiness (if self._cost_input and ...), so a cost of 0.0 suppresses the cost line. Use is not None for precision.

  • [file-permissions] src/run_log.py — Log file at /tmp/code-to-docs-run.jsonl uses default 0644 permissions. Defense-in-depth: use 0600 when include_prompts=True.

  • [code-organization] src/telemetry.py — Magic number 1_000_000 in cost calculation could be a named constant per the project's pattern in config.py.

  • [documentation-clarity] src/run_log.py — Class docstring says "Append-only JSONL log" but __init__ deletes existing files on each run.


Labels: PR adds observability features (token tracking, cost estimation, run logs) to the GitHub Action


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread action.yml
@@ -81,6 +93,8 @@ outputs:
description: 'JSON array of modified files'
pr-created:
description: 'Whether a PR was created'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high] api-contract

The action declares an acceptance-rate output but entrypoint.sh never writes acceptance-rate=... to $GITHUB_OUTPUT. The acceptance rate is only printed to stdout in suggest_docs.py. Downstream workflows referencing steps..outputs.acceptance-rate will always get an empty string.

Suggested fix: Write the acceptance rate to GITHUB_OUTPUT in suggest_docs.py (e.g., append acceptance-rate= to os.environ.get('GITHUB_OUTPUT')).

Comment thread src/suggest_docs.py
cost_input = os.environ.get("COST_PER_1M_INPUT", "")
cost_output = os.environ.get("COST_PER_1M_OUTPUT", "")
usage_tracker = UsageTracker(
cost_per_1m_input=float(cost_input) if cost_input else None,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] error-handling-gap

float(cost_input) will crash with ValueError on non-numeric input (e.g., '$3.00'). The existing get_max_context_chars() in config.py uses try/except with warning and fallback.

Suggested fix: Wrap each float() call in try/except ValueError, log a warning, and default to None.

Comment thread src/telemetry.py

if self._cost_input and self._cost_output and all_reported:
cost = (total_prompt / 1_000_000) * self._cost_input + (
total_completion / 1_000_000

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] logic-error

Total row in format_summary() shows all calls but only sums tokens from stages that reported usage, making totals misleading when some stages lack usage data.

Comment thread src/telemetry.py
) * self._cost_output
lines.append(f"\nEstimated cost: ${cost:.4f}")

table = "\n".join(lines)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] edge-case

Cost condition uses truthiness (if self._cost_input and ...), so a cost of 0.0 suppresses the cost line. Use 'is not None' for precision.

@fullsend-ai-review fullsend-ai-review Bot added feature python Pull requests that update python code labels Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature python Pull requests that update python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant