Improves/Fixes judge prompt; testing for templates

[jtoman]

There were three problems with the judge template.

1. For whatever reason it was more than happy to wave through skip reasons that were bogus without any research. This (plus using a smarter model) made it start rejecting BS skip reasons that we observed in the wild [1]
2. More pressingly, we were incorrectly binding the parameters to the template so the properties weren't showing up for the judge (it had to work off the comments in the cvl spec). This was a confounding factor with poor coverage around the judge agent, but even fixing this, we still needed the prompt improvements of 1.
3. After compaction, the skips and rebuttals were completely gone. They have been pushed into the initial template, whose rendering has been deferred.

This is the second time that woopsies in our prompts has bitten us. I was tired of it. So I went a little crazy.

The other part of the PR

The Problem

We have a typing mechanism for the templates (TypedTemplate) and its cousin InjectedTemplate. InjectedTemplate was used to split the responsibility of who injected what template params, but it sacrificed type safety. We have yeeted it in favor of the stronger typed PartialTemplate.

However, we still have a problem; nothing actually ensures the parameters we pass into the template actually correspond to the values used to render the template, nor their shape. Turning on strict mode helps, but it doesn't provide that big assurance.

The Solution: Part 1

We now have a script composer/scripts/template_manifest.py which crawls our source to find all uses of TypedTemplate and PartialTemplate. This is done through a very shallow syntax check, but we have programmed in assurance around that.

This manifest is a static artifact in the repo; we have a test now that verifies recomputing from source matches that file. Further, we have another test that actually walks the whole module root and imports every single module (with some exceptions). During this walk we instrument the TypedTemplate and PartialTemplate constructors to record every instance they create; if we find that the templates named in the manifest don't cover all of the instances created during import, we fail the test. [2]

Finally, we have another sanity test that all of the templates named in the manifest, once resolved, can have their template parameters extracted via reflection.

Why do we need all this? Well...

The Solution: Part 2

We can use this manifest and the runtime recoverable template parameter types to fuzz the templates. This is found in test_fuzzed_templates.py

The test loads the manifest and then, for each template, fuzzes the template. We resolve the type parameters, and use "hypothesis" to construct correctly shaped dict instances for the declared template type. We then render the template under strict rendering, which throws an exception if we reference an undefined name/field/attribute/key/whatever.

However, there were several things we had to work around which makes the resulting test file look like hell on earth.

One is that "hypothesis" does not understand pydantic's Field(pattern=....) so we got bogged down in it generating tons of solidity identifiers that did not match the regex. That is worked around by hooking in (via monkey patching, sigh) to the internal _from_type function used by hypothesis. There is no better way (that I could find) to intercept all value generation calls that didn't require us cluttering up the actual main code.

A similar issue occurred with Field(default_factory=list); something about this confused "hypothesis" which made it pass some pydantic internal sentinel value something or other to the model validator, which complained about not receiving a list. Accordingly, we simply strip all metadata from pydantic fields when generating those types.

Another "fun" issue was in the constraint that every Application, SourceApplication etc. is expected to have at least one ExplicitContract-typed element in components. hypothesis kept generating examples where this list was empty, or the elements were all ExternalActor subtypes. There was no way to express (AFAICT) a generic rule like this, so we fallback on some pretty gnarly introspection to try to figure out what types belong in components and that there is at least one (correct) subtype of ExplicitContract. We do a similar trick for ContractComponent.

One final bit of grossness: we would run into issues where sort and the "shape" of context when calling the application_context templates wouldn't be in sync. It's an implicit assumption that when sort == update, the ContractInstances all contain subtypes of FromSourceContract. But these (common) template parameters live alongside other, template specific parameters. The solution is to add a @component_context decorator which is applied to any template param type which expects to use application_context_new. When loading a type with this decorator, we introspect the type of the sort field to find out its possible values, and then vary the types we generate depending on the result.

This all composes nicely and recursively, but we have to reach pretty deep into the hypothesis internals to get there.

The good news is that this trouble did in fact catch some sloppiness in our templates, including where we were conditioning on sort in application_context_new.j2 when that wasn't even bound. We've also now changed loosey-goosey "truthy" checks to explicit is defined.

[1] Examples of BS reasons: "CVL can't support hooks on struct fields packed within the same slot" (obviously false), or "You can't do this when you use a NONDET summary for this function" (when the author claiming the skip CHOSE to write nondet summary; i.e., they tied their own hands and then claimed "can't do it, my hands are tied")
[2] This obviously doesn't catch cases where TypedTemplate instances are created dynamically within some function call, but there are limits to even my powers.

[shellygr]

prompt comments

[shellygr]

this gives an easy way out for the author agent out of Ground A type skips: it may say that it cannot modify a summary (e.g. NONDET) because otherwise the run would time out.

I am not saying to change the prompts more but it points to further work we'll need to do

[shellygr]

something something grounding? which types of dynamic-length arrays? memory or storage?
worth clarifying.

I suppose it's important enough that you put it in the main prompt

[shellygr]

I appreciate the indexing of all prompts actually

[jtoman]

It's not all prompts, just those that use our typed interface but yeah. Ideally we'd have this used for every template

[ericeil]

This is very, very cool! I would love to see more documentation of how the tests work (which I guess is the core of this change), and a CI test to verify that the manifest is up to date.

Also, as mentioned in Slack, we should consider whether we could re-use the template engine I'm planning to use for the Rust code, which might obviate the need for this (though of course it would come with its own problems).

[ericeil]

Remove this comment?

[jtoman]

Haha this definitely isn't copy pasted from a stub I asked Claude to write bad_poker_face.gif

[ericeil]

So I guess the idea is to run this any time we make a change that would affect the manifest? Can we have a CI workflow that checks that the manifest is up-to-date?

[jtoman]

It exists brother. test_manifest or something

[ericeil]

Please add some comments about the difference between TypedTemplate and PartialTemplate, when to use each, etc.

[ericeil]

These changes should probably be in a separate PR (and maybe reviewed by folks more familiar with the problem this addresses).

[ericeil]

This is hard for me to follow. Can you add some documentation describing how this works (and what a "cursed patcher" is? 😄 )

[ericeil]

This really needs some descriptive comments also