fix(diff --application): an unobserved agent is not "no change" (#876)

[pengfei-threemoonslab]

Closes #876.

This PR now carries a different implementation from the one it opened with. The original commits (cbc82a8, 2892a3c) conflicted with main after #864 (#879), and a review found two P2 regressions in them: a conditionally defaulted tools=None builder parameter and a global tools rebinding. Each let a real SDK tool addition print compared with no rows.

Three candidates were compared against main 1b1f646, each rebased onto main:

• A: this PR's original.
• B: the second implementation on claude/issue-876-silent-compared.
• R: B after 25 further local review rounds. It was unpushed; it is now preserved at claude/issue-876-round25-preserved.

This branch carries B, plus fixes (F). Why not the others:

• A still printed compared with no rows on 11 probes that B catches, among them dataclasses.replace/copy.replace with tools=, a .clone(tools=…) of an imported agent, a type()-made class, and tools changed after construction.

• R catches about 145 more changed-after-construction shapes. But:

  • It is about 3× the code (+3,540 source lines against about 1,120).
  • It is 4.5× slower on a scope with no agents.
  • A very common pattern, client.chat.completions.create(..., tools=req.tools) in any handler in scope, turns every agent's rows into not_established.

  Those shapes can return as a follow-up once R's precision and cost are fixed.

Problem

On kkmiecik-coder/CRM#5, diff --application answered compared with no rows, although the Wycena agent gained wyslij_obraz:

• The SDK reader skipped return Agent(...) builders.
• A test double established the scope.

Separately, one tool name defined twice in a test file made the whole comparison refuse (exit 2).

What changes

• Every OpenAI Agents SDK construction is read, or named as a limit.

  • Read, by their literal name=: return Agent(...), self.agent = Agent(...), inline agents, Agent[Ctx](...) and positional names.
  • Named as a limit on their file:
    • **/extra positional arguments;
    • clone/replace copies with their own tools;
    • tools changed after construction;
    • SDK and Google ADK agent subclasses that something instantiates or imports;
    • one identity built twice with different tools.

  A compared result therefore holds no unaccounted construction site.

• Identical Google ADK constructions of one name are one agent, as on main. Differing ones are a named limit.

• Test files (discovery's test-path convention, relative to the scope) are not read as the application. Each side lists them in a new additive field, excluded_tests, which the text output also prints. The application comparison result stays at the unreleased 0.1.

• A tool name defined twice is a gap on that name in its file. It no longer refuses the comparison. A tool that both constructions of one identity bind the same way keeps its ADDED row, as in the tensorflow#128063 shape.

• No agent source, no census. A scope where neither side has an SDK or ADK source stays not_established.

• Surfaces. The application_diff row in docs/distribution-surfaces.md names excluded_tests and the unread-construction limits. docs/application-comparison.md and CHANGELOG are updated.

• scan impact (intended; stated in CHANGELOG):

  • A Google ADK agent subclass in use, or one agent name built twice with different bindings, moves an ADK module from passed to insufficient_evidence.
  • SDK verdicts do not move: they are already insufficient_evidence on main, because the SDK reader's confidence is capped at medium. What changes is the selected gap.

What it does not do

• Still compared with no rows (a follow-up is suggested):
  • ADK: root_agent.clone(update={"tools": …}), model_copy(update=…), LlmAgent(**CFG), LlmAgent.model_validate({...}), and an ADK list handed to a helper that appends to it.
  • SDK: functools.partial(Agent, tools=…), an Agent re-exported through a project module, getattr(agents, "Agent")(…), and changes made through importlib.import_module(...).
• Changes after construction through helpers, namespaces (vars(a), __dict__, object.__setattr__), computed names (setattr(a, key, …)), or handles moved out of view are neither read nor named. R catches these.
• It can say partial too readily. x.tools = … in an application module that imports from the scope is a limit even when x is not an agent, for example an OpenAI request object.
• CRM's imported list stays an unresolved-tool limit on Wycena under --scope app. That is Application diff reports 'compared, no changes' when an SDK agent built by 'return Agent(...)' gains a tool #876's named fallback, not an ADDED row.
• Corpus numbers not re-measured. This PR's original numbers (134 open + 48 modify-existing: compared 3 → 0, exit 2 3 → 0) came from the pre-Google ADK: resolve repository-local imported functions and module-qualified tool bindings #864 implementation A, so they do not describe this tree.

Validation (4d76558, on main 107878b)

• tests/test_application_diff*.py, test_google_adk, test_imported_tool_review, test_python_import_resolution, test_imported_tool_bindings, test_distribution_surface_parity, test_docs_links: 831 passed, 1 skipped.
• 23 scan-side suites: 711 passed.
• tests/test_latency_budget.py -m perf on its own: 5 passed. This now includes the SDK reader scaling check, which was moved out of the xdist-loaded gate after it failed there under load.
• ruff check . and git diff --check: clean.
• 150 probe cases against main: 53 → 25 silent cases, and the remaining set is a subset of main's.
• Timing matches main at --scope src: 10.7s on both.

Implementation selection, fixes and review by a coding agent (Claude Code), not an independent human approval.

🤖 Generated with Claude Code

[pengfei-threemoonslab]

Reviewed commit 2892a3c. I recommend fixing the two P2 regressions in the inline comments before merging: both allow a real SDK tool addition to produce comparison_status: compared, rows: [], and no coverage gaps.

Validation: the application comparison suites (tests/test_application_diff*.py), Google ADK suite, SDK boolean-source and guard-dependency suites, and input-loader suite all passed locally. I also reproduced both findings through the public diff --application CLI against temporary Git repositories and compared the same inputs with main (6394ffd). The first case changes main's not_established into a false complete comparison; the second loses an added execute row that main correctly emits.

Agents Shipgate verification of the PR refs reports control_state: complete and release decision passed, refreshed after the required boundary-check route. That static gate result does not detect the behavioral regressions below. No source changes were made.

This is submitted as a comment review because the authenticated reviewer account is also the PR author.

[pengfei-threemoonslab]

[P2] Keep conditionally defaulted parameters unresolved

The literal-list lookup runs before the parameter check, so a common builder pattern bypasses the new dynamic-parameter protection:

def build(tools=None):
    if tools is None:
        tools = [lookup]
    return Agent(name='Built', tools=tools)
app = build([lookup, execute])

With local @function_tool definitions for lookup and execute, changing the caller from [lookup] to [lookup, execute] yields compared, no rows, and no coverage gaps on this head. The observation incorrectly says tool_names=['lookup'], tools_complete=True; main returned not_established for this unsupported builder. The conditional fallback is not evidence of the caller-supplied list. Check parameter ownership before using a cached literal (or prove the assignment reaches the construction) and preserve a dynamic-tools gap when the passed value is unresolved.

[pengfei-threemoonslab]

[P2] Account for global/nonlocal writes when indexing literal lists

Assignments are now indexed by their lexical scope, whereas reads use binding_scope(), which honors global/nonlocal. Those keys disagree for a write to an outer list. For example, with local decorated tools:

tools = [lookup]
def build():
    global tools
    tools = [lookup, execute]
    agent = Agent(name='Built', tools=tools)
    return agent
app = build()

Comparing an inner assignment of [lookup] against [lookup, execute], main emits ADDED agent -> execute; this PR instead returns compared, no rows, and no gaps. The inner assignment is stored under the function scope, then the constructor reads the stale module entry. Resolve assignment ownership consistently with reads, or mark such outer-scope writes unresolved rather than claiming the module initializer is the complete binding list.